Fix rdr2geo and geo2rdr lines_per_block bug (#996)

* Add getter/setter for geor2rdr processing params

* Avoid linesPerBlock to be overriden

* Avoid overrid of linesPerBlock in CUDA topo

* Add getter/setter for linesPerBlocks in CUDA Topo

* Fix issues due to shadowing of _linesPerBlock member in Geo2rdr/Topo

* Remove `_linesPerBlock` member variable in derived classes that
  shadowed the member with the same name in the base class, leading to
  unexpected behavior
* Replace instances of private base member `_linesPerBlock` in derived
  classes with `linesPerBlock()` getter method instead
* Remove `computeLinesPerBlock()` methods
* Remove commented code

Co-authored-by: Geoffrey M Gunter <[email protected]>

Author: 2 people

cxx/isce3/cuda/geometry/Geo2rdr.cpp

@@ -102,12 +102,9 @@ geo2rdr(isce3::io::Raster & topoRaster,
     // Interpolate orbit to middle of the scene as a test
     _checkOrbitInterpolation(tmid);
 
-    // Compute number of lines per block
-    computeLinesPerBlock();
-
     // Compute number of DEM blocks needed to process image
-    size_t nBlocks = demLength / _linesPerBlock;
-    if ((demLength % _linesPerBlock) != 0)
+    size_t nBlocks = demLength / linesPerBlock();
+    if ((demLength % linesPerBlock()) != 0)
         nBlocks += 1;
 
     // Cache near, mid, far ranges for diagnostics on Doppler
@@ -121,11 +118,11 @@ geo2rdr(isce3::io::Raster & topoRaster,
 
         // Get block extents
         size_t lineStart, blockLength;
-        lineStart = block * _linesPerBlock;
+        lineStart = block * linesPerBlock();
         if (block == (nBlocks - 1)) {
             blockLength = demLength - lineStart;
         } else {
-            blockLength = _linesPerBlock;
+            blockLength = linesPerBlock();
         }
         size_t blockSize = blockLength * demWidth;
 
@@ -194,33 +191,3 @@ _checkOrbitInterpolation(double aztime) {
     isce3::core::cartesian_t satxyz, satvel;
     this->orbit().interpolate(&satxyz, &satvel, aztime);
 }
-
-// Compute number of lines per block dynamically from GPU memory
-void isce3::cuda::geometry::Geo2rdr::
-computeLinesPerBlock() {
-
-    // Compute GPU memory
-    const size_t nGPUBytes = getDeviceMem();
-
-    // 2 GB buffer for safeguard for large enough devices (> 6 GB)
-    size_t gpuBuffer;
-    if (nGPUBytes > 6e9) {
-        gpuBuffer = 2e9;
-    } else {
-        // Else use 500 MB buffer
-        gpuBuffer = 500e6;
-    }
-
-    // Compute pixels per Block (3 double input layers and 2 float output layers)
-    size_t pixelsPerBlock = (nGPUBytes - gpuBuffer) /
-                            (3 * sizeof(double) + 2 * sizeof(float));
-    // Round down to nearest 10 million pixels
-    pixelsPerBlock = (pixelsPerBlock / 10000000) * 10000000;
-
-    // Compute number of lines per block
-    _linesPerBlock = pixelsPerBlock / this->radarGridParameters().width();
-    // Round down to nearest 500 lines
-    _linesPerBlock = (_linesPerBlock / 500) * 500;
-}
-
-// end of file

cxx/isce3/cuda/geometry/Geo2rdr.h

@@ -47,10 +47,6 @@ class isce3::cuda::geometry::Geo2rdr : public isce3::geometry::Geo2rdr {
                      const std::string & outdir,
                      double azshift=0.0, double rgshift=0.0);
 
-    private:
-        // Default number of lines per block
-        size_t _linesPerBlock = 1000;
-
     private:
         /** Print extents and image info */
         void _printExtents(pyre::journal::info_t &,
@@ -60,7 +56,4 @@ class isce3::cuda::geometry::Geo2rdr : public isce3::geometry::Geo2rdr {
 
         /** Check we can interpolate orbit to middle of DEM */
         void _checkOrbitInterpolation(double);
-
-        /** Compute number of lines per block dynamically from GPU memmory */
-        void computeLinesPerBlock();
 };

cxx/isce3/cuda/geometry/Topo.cpp

@@ -52,7 +52,7 @@ topo(Raster & demRaster,
 
     // Initialize a TopoLayers object to handle block data and raster data
     TopoLayers layers(outdir, radarGrid.width(), radarGrid.length(),
-            _linesPerBlock, this->computeMask());
+            linesPerBlock(), this->computeMask());
 
     // Call topo with layers
     topo(demRaster, layers);
@@ -102,7 +102,7 @@ topo(Raster & demRaster, Raster * xRaster, Raster * yRaster, Raster * heightRast
 
     // Initialize a TopoLayers object to handle block data and raster data
     // Create rasters for individual layers (provide output raster sizes)
-    TopoLayers layers(_linesPerBlock, xRaster, yRaster, heightRaster, incRaster,
+    TopoLayers layers(linesPerBlock(), xRaster, yRaster, heightRaster, incRaster,
             hdgRaster, localIncRaster, localPsiRaster, simRaster,
             maskRaster);
 
@@ -135,12 +135,9 @@ topo(Raster & demRaster, TopoLayers & layers) {
     // Create a DEM interpolator
     DEMInterpolator demInterp(-500.0, this->demMethod());
 
-    // Compute number of lines per block
-    computeLinesPerBlock(demRaster, layers);
-
     // Compute number of blocks needed to process image
-    size_t nBlocks = radarGrid.length() / _linesPerBlock;
-    if ((radarGrid.length() % _linesPerBlock) != 0)
+    size_t nBlocks = radarGrid.length() / linesPerBlock();
+    if ((radarGrid.length() % linesPerBlock()) != 0)
         nBlocks += 1;
 
     // Cache near, mid, far ranges for diagnostics on Doppler
@@ -154,11 +151,11 @@ topo(Raster & demRaster, TopoLayers & layers) {
 
         // Get block extents
         size_t lineStart, blockLength;
-        lineStart = block * _linesPerBlock;
+        lineStart = block * linesPerBlock();
         if (block == (nBlocks - 1)) {
             blockLength = radarGrid.length() - lineStart;
         } else {
-            blockLength = _linesPerBlock;
+            blockLength = linesPerBlock();
         }
 
         // Diagnostics
@@ -214,39 +211,6 @@ topo(Raster & demRaster, TopoLayers & layers) {
          << pyre::journal::newline;
 }
 
-// Compute number of lines per block dynamically from GPU memory
-void isce3::cuda::geometry::Topo::
-computeLinesPerBlock(isce3::io::Raster & demRaster, TopoLayers & layers) {
-
-    // Compute GPU memory
-    const size_t nGPUBytes = getDeviceMem();
-
-    // Assume entire DEM passed to GPU
-    const size_t nDEMBytes = demRaster.width() * demRaster.length() * sizeof(float);
-
-    // 2 GB buffer for safeguard for large enough devices (> 6 GB)
-    size_t gpuBuffer;
-    if (nGPUBytes > 6e9) {
-        gpuBuffer = 2e9;
-    } else {
-        // Else use 500 MB buffer
-        gpuBuffer = 500e6;
-    }
-
-    // Compute pixels per Block (4 double and 5 float output topo layers)
-    size_t pixelsPerBlock = (nGPUBytes - nDEMBytes - gpuBuffer) /
-                            (4 * sizeof(double) + 5 * sizeof(float));
-    // Round down to nearest 10 million pixels
-    pixelsPerBlock = (pixelsPerBlock / 10000000) * 10000000;
-
-    // Compute number of lines per block
-    _linesPerBlock = pixelsPerBlock / this->radarGridParameters().width();
-    // Round down to nearest 500 lines
-    _linesPerBlock = (_linesPerBlock / 500) * 500;
-    // Make sure we don't exceed number of lines in topo rasters
-    _linesPerBlock = std::min(_linesPerBlock, layers.length());
-}
-
 // Compute layover and shadow masks
 void isce3::cuda::geometry::Topo::
 _setLayoverShadowWithOrbit(const Orbit & orbit,

cxx/isce3/cuda/geometry/Topo.h

@@ -55,13 +55,6 @@ class isce3::cuda::geometry::Topo : public isce3::geometry::Topo {
         void topo(isce3::io::Raster &, isce3::geometry::TopoLayers &);
 
     private:
-        // Default number of lines per block
-        size_t _linesPerBlock = 1000;
-
-        // Compute number of lines per block dynamically from GPU memmory
-        void computeLinesPerBlock(isce3::io::Raster &,
-                                  isce3::geometry::TopoLayers &);
-
         // Generate layover/shadow masks using an orbit
         void _setLayoverShadowWithOrbit(const isce3::core::Orbit & orbit,
                                         isce3::geometry::TopoLayers & layers,