-
Notifications
You must be signed in to change notification settings - Fork 218
test_scheduler: Move case.cmpgen_namelists call to _sharedlib_build_phase #4819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test_scheduler: Move case.cmpgen_namelists call to _sharedlib_build_phase #4819
Conversation
Calling it in _setup_phase (as before) results in a namelist build failure for custom SystemTests that need to set some values that have no defaults.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4819 +/- ##
=========================================
Coverage ? 55.40%
=========================================
Files ? 266
Lines ? 38449
Branches ? 8307
=========================================
Hits ? 21304
Misses ? 14763
Partials ? 2382 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@jedwards4b Before I work on fixing the system tests, is this something you'd be amenable to bringing in? Or is there a better way you think I should handle this issue? |
|
@samsrabin I think that this is okay as long as you can fix what it breaks. |
|
@jedwards4b I've fixed the failing system test, although I don't really understand how. Basically I needed to add back the call of |
|
@samsrabin @jgfouca writes: this PR appears to have broken some stuff for us (E3SM). We have tests that are showing NML diffs and regenerating them is not fixing the issue. [T]he issue is that the nml bless/generate does not do the build phase, but the real run now does the nml later, so they won't ever match. |
|
Hmmm, now that you mention it, this might have caused similar issues in our CTSM tests when we first updated to cime6.1.112. @jgfouca, are there any meaningful namelist differences shown in the TestStatus.log files, or are they seemingly empty like this? If it's just like that, it should just be a one-time thing—tags after the initial merge of cime6.1.112 did not have this issue. For my reference:
|
|
@samsrabin , as an example for the case |
|
Ah, well I guess that would have been too easy. @jedwards4b I might need your help for this one; will message you separately. |
|
I can help but I think that I need more clues. Can this problem be reproduced in any ERP test? |
|
@jedwards4b , since ERP fiddles with pe settings that impact namelists, i think it should be reproducible for any ERP test (or PET, etc) |
As part of ESCOMP/CTSM#3292, I made two SystemTests that exercise a command (
subset_data) to generate CTSM input data and then run with the results. That command is exercised in the SystemTest's custombuild_phase(), because it's the earlier of the two methods inherited fromSystemTestsCommonthat we're allowed to override.Given the compset used by those tests, there are certain variables (at least
fsurdat) that have no default. The tests worked fine when I wasn't comparing against or generating a baseline. However, when I did, I would get namelist build failures because the default was missing and the custom setting hadn't yet happened.TestStatus:And the end of
TestStatus.log:I solved the problem by moving the call of
case.cmpgen_namelistsintest_schedulerfrom_setup_phase()to_sharedlib_build_phase(). So nowTestStatusis all green:Note the
NLCOMPstep coming after bothBUILDsteps, instead of afterSHAREDLIB_BUILDlike it usually does. I'm sure this will cause problems in the test suite because of that rearrangement; I'll plan to fix those if y'all are amenable to this solution. (I will also update to the latest CIME tag.)Other solutions I considered:
subset_dataduring theSUBSETDATAPOINT__init__()method. That didn't work, seemingly because noSUBSETDATAPOINTobject is initialized until after the problematic point intest_scheduler.user_nl_clmand whatever else duringSUBSETDATAPOINT__init__(). Won't work for the same reason as above.Other ideas I could try if this PR is rejected:
Test suite:
Test baseline:
Test namelist changes:
Test status: [bit for bit, roundoff, climate changing]
Fixes [CIME Github issue #]: None
User interface changes?: None
Update gh-pages html (Y/N)?: No