Solve CI failures - #366
Conversation
Layers whose raster was never requested (e.g. the ground-to-satellite east/north components, which nisar.workflows.rdr2geo always disables) are left with size-0 valarrays by setBlockSize. writeData() unconditionally took &valarray[0] for every layer to build the variant list, which is out-of-bounds when the valarray is empty. This undefined behavior aborts under libstdc++ builds with bounds-checked valarray::operator[], crashing the Python interpreter during test.python.pkg.nisar.workflows.rdr2geo. Only take the address when the valarray is non-empty; the existing per-raster nullptr guard in the write loop already lines up with each valarray's resize condition. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The LUT2d-Doppler overload of geo2rdr uses the incoming aztime value as its initial azimuth time guess rather than always performing its own coarse search. The test left aztime uninitialized, so it relied on whatever garbage happened to be on the stack; that value must fall within the orbit's time span and the Doppler LUT2d's valid domain, or the very first iteration fails with an out-of-bounds lookup. This was silently working by chance until a toolchain change altered the stack layout, causing OutOfBoundsLookup and a spurious test failure. Initialize aztime to orbit.midTime(), which is guaranteed to satisfy both constraints. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
est_xyz_eaz/eoa/aoe/tp were reserve()'d instead of resize()'d before being indexed, leaving size() at 0. Newer libstdc++ bounds-checking in operator[] catches this and aborts test.cxx.isce3.antenna.frame.
|
There were two dereferencing bugs caught thanks to the newer compilers/runtime libraries. One was in the isce3 library while the other was just in the unit test implementation. There was another bug in the geo2rdr test related to using an uninitialized aztime. The code actually does check whether the time falls inside the start/end time of the orbit and has a strategy to deal with that. However, in this case the extent of the LUT2d doesn't cover the full orbit, it's not set to extrapolate, and the compiler happened to set the aztime to a value inside the orbit but outside the LUT2d. Setting the aztime guess explicitly in the test prevents the failure. Now it's just the macos debug build that's failing. |
* sm r3.4 * change the SM commit id for R4.0.2 * update the SM SAS version to v0.4.3 --------- Co-authored-by: Xiaodong Huang <xhuang@nisar-adt-dev-3.jpl.nasa.gov>
dryTropoDelayTSX() binds Eigen (Vec3) arguments but this file never included pybind11/eigen.h, so Vec3 resolved to the generic type_caster_base here instead of the Eigen/numpy specialization used elsewhere (e.g. slantRangeFromLookVec in pntintersect.cpp, which shares the exact same argument types). That is an ODR violation: two translation units instantiate the same pybind11 internal template with genuinely different content. It was apparently harmless under pybind11 2.x, but crashes import on pybind11 3.x with an internal type-signature-parsing error. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This reverts commit b4559f4.
|
Finally, success! There actually wasn't a problem in pybind11, but the update merely exposed a problem in the isce3 bindings. Basically two methods had the argument list I must say, after a period of relative complacency, this episode really reminded me of what I don't like about C++! |
There was a problem hiding this comment.
I'm not sure why this change from #349 is showing up in the diff...
There was a problem hiding this comment.
It looks like commit a1c757b from that PR somehow ended up in your git history for this one. Maybe a rebase would fix it?
3 participants
CI is failing for every PR right now. I've narrowed the problem with
test.python.pybind.geometry.pntintersectto the latest version of pybind11 (v3.1.0), while the previous version (3.0.4) seems to work okay. Claude thinks it has something to do with the newcall_implmethod, but I didn't go too deep down that rabbit hole. Changing the function signature in the Python binding ofslantrange_from_lookvecto pass the ellipsoid by value instead of byconst&seems to work around the issue, but that seems like a hack.For now I just blacklisted the latest pybind11 version in hopes it gets fixed upstream.Edit: As noted below, it's not actually necessary to blacklist the latest pybind11 version.I think there may be some other CI failures as well and will use this branch to work on them.