AtlasFieldCache try to add more/improve comments

MagneticField/MagFieldElements/MagFieldElements/AtlasFieldCache.h

 /*
-  Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2022 CERN for the benefit of the ATLAS collaboration
 */
 
 /**
@@ -31,6 +31,12 @@ namespace MagField {
  *  @brief Local cache for magnetic field (based on
  * MagFieldServices/AtlasFieldSvcTLS.h)
  *
+ *  We keep track of the
+ *  - magnetic field map
+ *  - solenoid/toroid scales from the currents
+ *  - The current zone
+ *  - The current/cached cell in the zone.
+ *
  *  @author R.D.Schaffer -at- cern.ch
  */
 class AtlasFieldCache
@@ -38,7 +44,7 @@ class AtlasFieldCache
 public:
   AtlasFieldCache() = default;
   /** constructor to setup with field scale
-   * and magnetic field service for
+   * and magnetic field map for
    * first access to field */
   AtlasFieldCache(double solFieldScale,
                   double torFieldScale,
@@ -63,7 +69,7 @@ public:
    * works only inside the solenoid.
    * Otherwise call getField above.
    * xyz[3] is in mm, bxyz[3] is in kT
-   * if deriv[9] is given, field 
+   * if deriv[9] is given, field
    * derivatives are returned in kT/mm
    * */
   void getFieldZR(const double* ATH_RESTRICT xyz,
@@ -83,34 +89,40 @@ private:
   /// fill Z-R cache for solenoid */
   bool fillFieldCacheZR(double z, double r);
 
-  /// Full 3d field
-  BFieldCache m_cache3d;
-
-  /// zone of full 3d field cache  
-  const BFieldZone* m_zone3d { nullptr };
-
-  /// Fast 2d field
-  BFieldCacheZR m_cacheZR;
-
   /// magnetic field scales from currents
   double m_solScale{ 1 };
   double m_torScale{ 1 };
   double m_scaleToUse{ 1 };
 
-  /// handle to the magnetic field service - not owner
+  // Solenoid zone ID number - needed to set solScale.
+  // Assumes only one Solenoid zone.
+  int m_solZoneId{ -1 };
+
+  /// handle to the magnetic field map - not owned
   const AtlasFieldMap* m_fieldMap{ nullptr };
 
+  /// A zone of the full 3d field.
+  /// This can be solenoid or one of the
+  /// toroid etc zones
+  /// Owned by AtlasFieldMap
+  const BFieldZone* m_zone3d{ nullptr };
+
+  /// Full 3d field cell/cache
+  /// This will be a cell inside
+  /// a 3d field zone
+  BFieldCache m_cache3d;
+
   /// Pointer to the conductors in the current field zone (to compute
   /// Biot-Savart component) Owned by AtlasFieldMap.
   const std::vector<BFieldCond>* m_cond{ nullptr };
 
-  // fast 2d map (made of one zone)
+  /// fast 2d map made of one zone
+  /// assuming phi/roatational symmetry
   /// Owned by AtlasFieldMap.
   const BFieldMeshZR* m_meshZR{ nullptr };
 
-  // Solenoid zone ID number - needed to set solScale. Assumes only one Solenoid
-  // zone!
-  int m_solZoneId{ -1 };
+  /// Fast 2d field cell/cache
+  BFieldCacheZR m_cacheZR;
 };
 
 } // namespace MagField

MagneticField/MagFieldElements/MagFieldElements/AtlasFieldCache.icc

 /*
-  Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2022 CERN for the benefit of the ATLAS collaboration
 */
 inline MagField::AtlasFieldCache::AtlasFieldCache(double solFieldScale,
                                                   double torFieldScale,
                                                   const AtlasFieldMap* fieldMap)
   : m_solScale(solFieldScale)
   , m_torScale(torFieldScale)
-  ,
-  // set field service
-  m_fieldMap(fieldMap)
-
+  , m_fieldMap(fieldMap)
 {
   if (m_fieldMap) {
     // save ZR bfield
@@ -22,26 +19,26 @@ inline MagField::AtlasFieldCache::AtlasFieldCache(double solFieldScale,
 inline bool
 MagField::AtlasFieldCache::fillFieldCache(double z, double r, double phi)
 {
-  // Get a new zone from the map ONLY if we are no longer in the current zone
-  // (see problems outside soleniod for this logic, try for the moment to only optimize for solenoid,
-  //  id = 7000. RDS 2021/12)
-  if (!m_zone3d || m_zone3d->id() != m_fieldMap->solenoidZoneId() || !m_zone3d->inside( z, r, phi )) {
-    
-    // search for the zone
-      m_zone3d = m_fieldMap ? m_fieldMap->findBFieldZone(z, r, phi) : nullptr;
+
+  // Check if are still in the current zone.
+  //  We have only one solenoid zone with  known id (m_solZoneId).
+  //  So if the id matches we aee fine.
+  //  Otherwise check if we are still inside.
+  if (!m_zone3d || m_zone3d->id() != m_solZoneId ||
+      !m_zone3d->inside(z, r, phi)) {
+    // we need to try and a find a new zone
+    m_zone3d = m_fieldMap ? m_fieldMap->findBFieldZone(z, r, phi) : nullptr;
   }
-  
+
   if (!m_zone3d) {
-    // outsize all zones
+    // we failed to find a zone
     return false;
   }
 
   // set scale for field
   m_scaleToUse = (m_zone3d->id() == m_solZoneId) ? m_solScale : m_torScale;
-
   // fill the cache, pass in current scale factor
   m_zone3d->getCache(z, r, phi, m_cache3d, m_scaleToUse);
-
   // save pointer to the conductors in the zone
   m_cond = m_zone3d->condVector();
 

MagneticField/MagFieldElements/src/AtlasFieldCache.cxx

 /*
-  Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2022 CERN for the benefit of the ATLAS collaboration
 */
 
 //
@@ -32,7 +32,7 @@ constexpr double defaultB = 0.1 * Gaudi::Units::gauss;
 // to out-of-line Eigen code that is linked from other DSOs; in that case,
 // it would not be optimized.  Avoid this by forcing all Eigen code
 // to be inlined here if possible.
-__attribute__ ((flatten))
+__attribute__((flatten))
 #endif
 void
 MagField::AtlasFieldCache::getField(const double* ATH_RESTRICT xyz,
@@ -58,11 +58,13 @@ MagField::AtlasFieldCache::getField(const double* ATH_RESTRICT xyz,
   const double r = std::sqrt(x * x + y * y);
   const double phi = std::atan2(y, x);
 
-  // test if initialised and the cache is valid
+  // Check that the cached z,r, phi cell is valid
+  // and if we are still inside it
   if (!m_cache3d.inside(z, r, phi)) {
-    // cache is invalid -> refresh cache
+    // if not we need to find and cache a new cell
     if (!fillFieldCache(z, r, phi)) {
-      // caching failed -> outside the valid map volume
+      // caching failed
+      // outside the valid map volume
       // return default
       bxyz[0] = bxyz[1] = bxyz[2] = defaultB;
       // return zero gradient if requested
@@ -99,7 +101,7 @@ MagField::AtlasFieldCache::getFieldZR(const double* ATH_RESTRICT xyz,
   if (m_fieldMap == nullptr) {
     // return default
     bxyz[0] = bxyz[1] = bxyz[2] = defaultB;
-      // return zero gradient if requested
+    // return zero gradient if requested
     if (deriv) {
       for (int i = 0; i < 9; i++) {
         deriv[i] = 0.;
@@ -113,15 +115,14 @@ MagField::AtlasFieldCache::getFieldZR(const double* ATH_RESTRICT xyz,
   const double z = xyz[2];
   const double r = std::sqrt(x * x + y * y);
 
-  // test if the cache was initialized and the ZR cache is valid for current
-  // position
+  // Check that the cached z,r, cell is valid
+  // and if we are still inside it
   if (!m_cacheZR.inside(z, r)) {
-
-    // cache is invalid -> refresh cache
+    // cached cell is invalid -> refresh cached cell
     if (!fillFieldCacheZR(z, r)) {
-
-      // caching failed -> outside the valid z-r map volume
-      // call the full version of getField()
+      // No cell found -> outside the valid z-r map volume
+      // fallback to calling
+      // the full version of getField()
       getField(xyz, bxyz, deriv);
       return;
     }