Add CESM_NOT_PORTED machine + optimize case creating tests (to not do that) - #271
Open
manishvenu wants to merge 6 commits into
Open
manishvenu wants to merge 6 commits into
manishvenu wants to merge 6 commits into
Conversation
On a host CESM has no machine definition for, visualCaseGen falls back to the CESM_NOT_PORTED placeholder machine and CaseCreator refuses to create a case. CrocoDash previously walked into that wall: _run_create_newcase raised, the error was swallowed, and every later step that shells out to xmlchange or appends to a user_nl file failed against a caseroot that was never created. Treat CESM_NOT_PORTED as "configure, but don't execute" instead. Case derives self.do_exec from the machine name -- no new public argument, since machine already defaults to cime.machine -- and threads it through every call site that would otherwise shell into the case directory. Forcing extraction is untouched: it never talks to CIME, so the expensive download+regrid work still runs, and the case itself is recreated later on a ported machine from crocodash_case.yaml. The do_exec plumbing mirrors is_non_local: a BaseConfigurator property sourced from the registry's live Case, applied to XMLConfigParam/UserNLConfigParam outputs by the base configure(), so no configurator declares a ctor arg to carry it. It defaults to True (the opposite of is_non_local's False) so a configurator with no live Case -- direct construction in tests, deserialize() -- behaves exactly as it did before. UserNLConfigParam.apply() previously hardcoded do_exec=True, as did the two batched append_user_nl calls in ConditionsConfigurator.configure(), which is the one configurator that deliberately bypasses the base per-param loop to preserve its user_nl banner formatting -- and the one writing most of user_nl_mom, so missing it would have defeated the feature. Also stop swallowing case-creation failures. _create_newcase caught every exception, printed it, and continued, so a half-created case was indistinguishable from a working one until something much later tripped over a file case.setup never wrote (a missing user_nl_cpl surfacing inside CaseBundle, for instance). It now reverts the ccs_config edits and re-raises. The two existing is_non_local tests assert an exact xmlchange call signature, which now carries do_exec, so both are updated alongside five new tests covering the XML and user_nl paths in both directions. Note the no-exec path cannot be exercised on a ported machine: init_args_check validates machine against cime.machines, which won't contain CESM_NOT_PORTED there. The plumbing is unit-tested via a mocked registry/case; end-to-end verification needs a genuinely un-ported host.
|
📄 Preview your docs here: |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
create_newcase plus case.setup is ~6.5s of the ~10s it takes to stand up a CrocoDash case, and the default suite was paying it ten times over for fixtures that never look at a CIME artifact. Configure-without-executing already existed for un-ported machines, so reuse it: one fixture creates a real case, everything else skips CIME entirely. Case gains a do_exec argument. self.do_exec was derived solely from MACHINE == CESM_NOT_PORTED, and init_args_check rejects that machine name on a ported host, so the no-exec path was unreachable anywhere it could be tested. do_exec=False now opts into the same behaviour explicitly, and an un-ported machine still forces it off, so the two can't disagree. It is excluded from the state file: crocodash_case.yaml exists to be replayed on a ported machine, and carrying do_exec=False into the replay would defeat that. Two things blocked the path once it was reachable. Case.expt read RUNDIR off self._cime_case, which is None when the case was never created, so it raised AttributeError -- a real hole for un-ported users too, not just for tests; it now falls back to the path CIME would have derived. And the no-exec branch of _create_newcase announced "Machine is CESM_NOT_PORTED" regardless of why it was taken, which is now simply untrue for a ported machine. On the test side CrocoDash_case_factory defaults to do_exec=False, and the new ported_case fixture is the single case that runs CIME. Tests needing artifacts only CIME writes -- xmlquery, env_*.xml, the default user_nl files, SourceMods/, README.case -- read that one case, and mutating tests take a copy of it via ported_case_copy, which costs a directory copy rather than a second create_newcase. Copying a caseroot needs the original's own paths rewritten out of it, or the isolation is a fiction: CIME resolves xmlchange's replay log against CASEROOT in env_case.xml, so an xmlchange in an unrewritten copy appends to the *original* case's replay.sh -- the copy never records the change and the shared case gets polluted. CaseBundle.bundle() has the same problem via user_nl_mom's INPUTDIR. Doing the rewrite with ./xmlchange CASEROOT= is worse than useless, since that registers as a non-standard xmlchange and then shows up in every bundle diff, so _rewrite_paths substitutes the paths directly, skipping symlinks so it cannot write through ./xmlquery or Tools/ into the CESM installation. test_case_init_and_create_grid_input no longer asserts on README, which create_newcase writes and the grid-input step it covers does not; test_ported_case_has_cime_artifacts covers that instead, so the do_exec=True path stays tested. test_configure_forcings_invalid_function_overrides gets its own function-scoped case, having previously called configure_forcings() on a session-scoped one and left the extract_forcings directory wiped for whoever ran next. test_configure_forcings had an assertion that could only ever raise NameError on failure, never fail. Default suite goes from 119.8s to 66.8s, and the four case-heavy files from 103.4s to 50.1s, with one test added and no change in what passes. Two ~9s costs remain and are not fixture-level: CaseBundle's identify_non_standard_case_info builds a fresh reference case to diff against, which no test-side change can avoid. Note also that test_diff_CESM_cases_nodiff is weaker than it was, since two copies of one case make "no differences" closer to a tautology than two independently created cases did.
manishvenu
changed the title
manishvenu
marked this pull request as ready for review
August 19, 2026 21:32
The previous commit added a public do_exec argument to Case because init_args_check rejects CESM_NOT_PORTED on a ported host, which made the un-ported path unreachable anywhere it could be tested. That was the wrong conclusion: CESM_NOT_PORTED is visualCaseGen's own placeholder, not a CIME machine, so "machine not in cime.machines" was never the right gate for it and the argument was working around a self-inflicted restriction. CIME_interface selects the placeholder in _handle_machine_not_ported() when CIME cannot identify the host, and from then on it is the only machine the interface offers. Requesting it on a ported host therefore has to reproduce that state, or it is only half-applied. Two things go wrong otherwise, both verified rather than assumed: MACHINE's options are set from the real machine list by set_launcher_options() during initialize(), so _configure_launch's assignment dies with "CESM_NOT_PORTED not an option for MACHINE" -- swallowed by _configure_case, leaving a Case whose caseroot does not exist; and _is_non_local() compares the interface's real machine against the selected one, so every xmlchange would wrongly be treated as non-local. _emulate_not_ported() puts the interface into that state, which also makes init_args_check pass untouched -- cime.machines becomes [CESM_NOT_PORTED] and project_required gains the matching entry, so no validation change is needed at all. do_exec goes back to being derived solely from the machine, and is no longer part of the state file exclusion list because it is no longer an init arg. It replicates the handler's three identity assignments rather than calling it, because the handler also falls back to ~/scratch and ~/inputdata and creates them -- correct on a laptop with nothing better to use, but the host's own roots already exist here, and a test run should not create directories in $HOME as a side effect (it did, once, before this was noticed). Replication can drift from the original, so test_emulate_not_ported_matches_visualcasegen pins the two together, driving visualCaseGen's handler with $HOME redirected to tmp_path and comparing the identity fields. Confirmed to fail if the values are changed. Requesting the placeholder on a ported host now prints why no case was created. Without that, replaying a laptop-configured crocodash_case.yaml on Derecho -- which records machine, so it says CESM_NOT_PORTED -- would quietly produce another case CIME never ran, where before it raised ValueError. The error was more useful than silence. The fixtures take a ported flag instead of do_exec, and the un-ported path is now asserted directly rather than only used: test_not_ported_machine_configures_ without_cime is the complement of test_ported_case_has_cime_artifacts, checking that the grid inputs and state file are written and that nothing create_newcase or case.setup writes exists. Note what this changes about the earlier claim that the path could not be verified without a laptop: the path itself is now exercised on a ported host. What still cannot be reached here is the *trigger* -- unsetting NCAR_HOST is not enough, because visualCaseGen force-selects casper from the fqdn on a JupyterHub node before CIME is ever consulted. Full suite: 246 passed, 16 skipped, 1 pre-existing failure (test_case_integration_driver, full-suite-only, fails identically at baseline).
get_cesm_root_path hardcoded one personal install and ignored $CESMROOT whenever
it was on glade, so nobody else could run the tests that build a Case, and
pointing them at a different checkout meant editing the fixture. $CESMROOT now
wins, with the per-platform paths as fallbacks, which is what the TODO there
asked for.
A checkout is genuinely required for those tests rather than merely convenient,
and CESM_NOT_PORTED does not change that: CIME_interface asserts <cesmroot>/cime
exists, reads the CIME git tag, imports CIME, and parses compsets, grids and the
machine list out of the tree -- all before _retrieve_machines can conclude the
host is un-ported. Un-ported means CESM is present but this host has no machine
definition, which is what its own alert says ("You can still run visualCaseGen
as normal, but the final step of case creation is disabled"). So there is
nothing to fall back to when the tree is absent, and skipping is the honest
outcome. Making those tests run without a checkout would mean stubbing
CIME_interface, which would stop testing real compset and grid resolution.
Skipping in the session fixture covers every dependent test transitively, so no
per-test markers are needed. Measured with CESMROOT pointed at an empty
directory: 222 passed, 41 skipped, no errors or failures -- 25 tests move to
skipped, and the 238 that never build a Case are untouched. With a checkout
present the suite is unchanged: 246 passed, 16 skipped, plus the pre-existing
test_case_integration_driver failure.
Note 25, not the 63 quoted earlier from counting whole files: most tests in
those five files use mocks or fixtures that never reach CIME.
CESM_NOT_PORTED made an un-ported machine work, but it could not make an absent CESM work, because it is a conclusion reached from a checkout that has already been read: CIME_interface asserts <cesmroot>/cime exists, reads CIME's version, imports CIME, and parses component classes, models, physics, options, grids and compsets, and only then does _retrieve_machines() decide the host is un-ported. The placeholder substitutes five host-configuration fields; nothing substitutes the model catalogue. But CrocoDash needs far less of that catalogue than the catalogue contains. Of its non-validation uses, comp_classes is fixed by the compset long-name format, get_components_from_compset_lname is pure string work, and cime_output_root and machine are exactly what the placeholder already supplies. Everything else the catalogue is read for is validation -- checking a compset's physics and options and a grid name against what this CESM can build. With no checkout there is nothing to validate against, so skip that step rather than refuse to run. So cesmroot becomes optional. Omitting it selects _NoCesmCIME, forces machine to CESM_NOT_PORTED, and skips _configure_case entirely. Passing a real machine without a cesmroot is an error rather than a silent downgrade, since CIME is what defines every machine other than the placeholder. Grid input files, the state file and the forcing configuration are all still produced; forcing extraction never touched CIME to begin with. The trade is deliberate and worth being explicit about: a typo in a compset long name or a bogus grid name is no longer caught at configure time and will instead surface when the case is created on a ported machine. Case prints that its input was accepted unvalidated. A compset alias cannot be resolved at all in this mode, so a long name is required -- alias resolution is the one thing that genuinely needs the catalogue. Four things had to move for this to work, each verified by hitting it: - MB_ATTEMPT_ID was read back out of cvars in four places. It is a uuid CrocoDash generates itself, so session_id is now a plain attribute set in __init__ and the cvar mirrors it, rather than the reverse. Nothing reads cvars for it. - is_non_local was re-derived from self.cc at ten call sites. There is no CaseCreator without a checkout, so it is resolved once in __init__ and reused; with CIME never invoked there is nothing to be non-local to. - init_args_check validated grid names against cime.domains. The shim reports domains as None -- not an empty dict, which would make every grid look invalid -- and the three lookups are guarded on it. - visualCaseGen's xmlchange() and append_user_nl() read cvars["CASEROOT"] before they consult do_exec, so they raise rather than no-op with cvars empty. They cannot be called at all here. BaseConfigurator gains has_cesm, deliberately narrower than do_exec: on an un-ported machine that does have a checkout, apply() is still called so it prints the commands for creating the case by hand, which is that path's whole purpose. The xmlchanges skipped in case.py all derive from init args that the state file records, so a replay applies them. Full suite: 249 passed, 16 skipped, 1 pre-existing failure (test_case_integration_driver, full-suite-only, fails identically at baseline).
manishvenu
changed the title
manishvenu
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Descrip:
Lets CrocoDash configure a case on a machine CESM isn't ported to — or with no CESM checkout at all — and makes the test suite roughly twice as fast as a side effect. visualCaseGen already falls back to the
CESM_NOT_PORTEDplaceholder when CIME can't identify the host, butCaseCreatorthen refuses to create a case.Changes:
cesmrootis now optional.CESM_NOT_PORTEDalone couldn't get us here — reaching that placeholder already requires reading CIME, and it only patches host fields, not the model catalogue. So we skip some steps:_configure_casedoesn't run (it's pure validation, and without a checkout there's nothing to validate against), and_NoCesmCIMEsupplies the little CrocoDash actually needs —comp_classesandget_components_from_compset_lnameoff the compset long name,cime_output_root/machineoff the placeholder.Deliberate trade, and
Caseprints it: a typo in a compset long name or a bogus grid name is no longer caught at configure time and will surface when the case is created on a ported machine. A compset alias can't be resolved at all in this mode — alias resolution is the one thing that genuinely needs the catalogue — so a long name is required. Passing a real machine with nocesmrootis an error, not a silent downgrade.Test suite: one real CESM case instead of ten.
create_newcase+case.setupwas ~6.5 s of the ~10 s per case, paid ten times over for fixtures that never look at a CIME artifact. Now oneported_casefixture runs CIME and the rest ask forCESM_NOT_PORTED; mutating tests copy the ported case, which needs the original's paths rewritten out orxmlchangeappends to the original case'sreplay.sh. Full suite 121.6 s → 67.8 s, case-heavy files 71.6 s → 35.4 s.$CESMROOTnow takes precedence over a hardcoded personal install, and a missing checkout skips rather than fails: 238 of 263 tests need no CESM.Minor changes:
Case.exptraisedAttributeErrorwhen the case was never created — a real hole for un-ported users, not just tests.session_idwas read back out ofcvarsin four places despite CrocoDash generating it; now a plain attribute the cvar mirrors.is_non_localwas re-derived fromself.ccat ten call sites; resolved once in__init__._create_newcaseswallowed every exception, so a half-created case looked identical to a working one until something later tripped over a filecase.setupnever wrote. It now reverts itsccs_configedits and re-raises. This may surface previously-hidden failures loudly, which is the intent.bundle_casewon't work from a case CIME never created —CaseBundle.__init__runs./xmlquery. Configure and process forcings locally, then recreate on a ported machine viacreate_case_from_yaml.test_diff_CESM_cases_nodiffis weaker now: two copies of one case make "no differences" closer to a tautology.NCAR_HOSTisn't enough, because visualCaseGen force-selectscasperfrom the fqdn on a JupyterHub node.test_emulate_not_ported_matches_visualcasegenpins the emulated state against visualCaseGen's own handler instead, so they can't drift.