Fix numerical instability in Integral Collocation ODE Solver#439
Fix numerical instability in Integral Collocation ODE Solver#439Mohit-Lakra wants to merge 1 commit intoGeomScale:developfrom
Conversation
|
@vissarion , the issue should be resolved now with the latest changes. |
vissarion
left a comment
vissarion
left a comment
There was a problem hiding this comment.
regression tests are mentioned (e.g. test_instability) but not appear in this PR
|
The tests should be included in the PR for reproducibility of the issue. |
|
@vissarion |
Mohit-Lakra
force-pushed
the
Fix_integral_collocation
branch
2 times, most recently
from
13ec0f0 to
edb85cf
Compare
|
Hi @vissarion, just a quick update I've updated the PR to handle the "Minimal Build" environment (which lacks FFTW3 and ifopt) while ensuring the test runs correctly in full environments. Summary of Fixes: Regression Test: Added test/test_integral_collocation_stability.cpp to verify energy conservation and prevent future instability. Header Fixes (integral_collocation.hpp): Included "basis.hpp" and defined BOOST_MATH_USE_FFTW3 to resolve missing type errors. Guarded nlp_oracles includes with #ifndef DISABLE_NLP_ORACLES to prevent build errors when ifopt is missing. CMake Configuration: Wrapped the new test in if(NOT DISABLE_NLP_ORACLES) in test/CMakeLists.txt. Reason: This ensures the test is skipped during minimal CI builds (where FFTW3 is unavailable) but runs in full builds, resolving the CI failure. |
Mohit-Lakra
force-pushed
the
Fix_integral_collocation
branch
5 times, most recently
from
7f3d8de to
3d4cbd1
Compare
…cale#120) Add regression test for stability Fix CI build: Guard NLP oracle includes Fix CI build: Guard NLP oracle includes Add missing headers Improve build and run instructions for hpolytope-volume example (GeomScale#376) Fix RK4 stage weighting to keep amplitude bounded (GeomScale#375) Added test Restore upstream files — not part of this fix Restore README to upstream state
Mohit-Lakra
force-pushed
the
Fix_integral_collocation
branch
from
61c66d3 to
5f6860b
Compare
| #ifndef ODE_SOLVERS_INTEGRAL_COLLOCATION_HPP | ||
| #define ODE_SOLVERS_INTEGRAL_COLLOCATION_HPP | ||
|
|
||
| #include "basis.hpp" |
| // 5. Apply degree-doubling transformation | ||
| // The transformation takes the n Chebyshev transform coefficients c[i] | ||
| // and creates a complex polynomial of order 2n with coefficients a[i] | ||
| // and creates a complex polynomial of order 2n with coefficients a[i] |
| void set_coord(int i, double val) { coords[i] = val; } | ||
| }; | ||
|
|
||
| struct MockPolytope { |
There was a problem hiding this comment.
Why not using an Hpolytope and a point from volesti point class?
2 participants
This PR fixes the instability issues in IntegralCollocationODESolver (Issue #120).
After investigating the solver's behavior, I identified two distinct bugs that were causing the energy explosion:
Fix: I applied the correct (eta / 2.0) scaling factor to the integration step.
Fix: I moved the X_prev update to the start of the loop, ensuring the error metric actually compares the current state against the previous one.
Verification
I verified these fixes locally using a Harmonic Oscillator test case (x '′ =−x).
Before: The system was unstable, with energy growing unboundedly (radius >1.5 within 100 steps).
After: The solution is now stable and bounded (radius ≈1.0), confirming the physics are correct.
Closes #120