Conversation
Since tendencies now use TotalVerticalTransportPseudoVelocity, initialize to the same values as VerticalPseudoVelocity and TotalVerticalPseudoVelocity. Do a running total of errors instead of zeroing out each time.
|
Thanks @overfelt. Taking a look now. |
TestingThis fixes the
The two remaining failures:
Work directory: Posted by Claude Code on @xylar's behalf. The testing, analysis and wording above are AI-authored; please check them accordingly. |
|
@overfelt, I approved too soon. (Multitasking gone awry.) Would you be able to look at the new failing tests on Chrysalis? I don't know my way around the CTests you added well enough to understand the failures yet. I can give it a go if you can't. |
xylar
left a comment
There was a problem hiding this comment.
Both remaining failures from my testing comment look like test-constant fixes. I haven't rebuilt with these changes.
For TEND_SPHERE_TEST, the LInf in the spherical ExpectedFCTHNew (line 222) needs to become 3.0543973828844884e-05, the value from my run; the L2 already passes. The old value predates the regenerated sphere mesh: 1a564eb updated ExpectedFCTHProv and ExpectedFCTHInv, but FCTHNewInv was inf at the time.
Posted by Claude Code on @xylar's behalf. The testing, analysis and wording above are AI-authored; please check them accordingly.
| const auto HProv = TrHorzAdvOnC.GetHProv(); | ||
| const auto HNewInv = TrHorzAdvOnC.GetHNewInv(); | ||
|
|
||
| const Real ATol = 1.0e-10; |
There was a problem hiding this comment.
The single-precision failures are round-off, up to 1.5e-5 against expected values of 0 or ~1e-16. With RTol scaling those tiny values, this ATol sets the tolerance, and 1e-10 is too tight for floats. Making it precision-dependent like RTol at line 1979 would cover all five on Chrysalis:
const Real ATol = sizeof(Real) == 4 ? 1e-4 : 1e-10;That is needed here and at lines 1495, 1541 and 1672, or once at the top of the function.
There was a problem hiding this comment.
I tried to test in Chrysalis but can't access the platform and assumed it was down for maintenance. Maybe something wrong with my account that I need to fix.
There was a problem hiding this comment.
Chrysalis isn't down so indeed it seems like you need to check with @rljacob about getting your account reactivated once he's back.
Since tendencies now use TotalVerticalTransportPseudoVelocity, initialize to the same values as VerticalPseudoVelocity and TotalVerticalPseudoVelocity. Fixes #573
Do a running total of errors instead of zeroing out each time.
Checklist
Building
Testing
Perlmutter