Fix accurate AD summation in residual equations - #1035
Conversation
|
LGTM but |
|
Not sure if you just need to rebase/merge |
|
yeah, i plan to take a closer look soon as well. Might just need to merge main in. |
746d93a to
fe12add
Compare
|
@warrickball i rebased to main and checked the double_bh test case photo checksum locally. Hopefully it passes the test_suite now and we can merge. |
|
I've just launched a test on |
|
We've got different failures on different machines, possibly related to |
|
There's no smoking gun in the logs IMO. There's a minor convergence issue at model 639 with I guess try a local run with all inlists? 🤷 I don't know these tests at all. |
|
Interesting. I would definitely put this down to sensitivity. LLNL DANE might need the mod files regenerated to work as well on a partial test. but I'm not sure. If you share the terminal output with me for the |
|
All passing on bluebear! 🥳 |
I believe the accurate AD summation types in mesa's residual equations were just being assigned the result of a normal AD expression sum, so the (dp) cancellation had already happened before the compensated summation logic even runs. So the accurate AD summation was not really being leveraged at all. I would call this an inconsequential bug, but fixing it could yield benefits.
This pr fixes the accurate AD sums to accumulate term-by-term through the accurate type, so each add/subtract actually uses the compensation algorithm. I added this type to a couple other locations where it might make sense (summing sources in hydro energy and in RSP2 turbulent energy and luminosity equations), and I did some minor clean up.
I'm not sure if this will help mesa convergence or not, it could in principle, but the point remains: this branch makes the intended behavior on main actually happen in practice. We will have to test and see if any thing changes in practice...