Skip to content

Fix critical bugs in sgh_restart_ensemble test case#10

Merged
alexolinhager merged 2 commits into
sgh_ensemble_generator_templatefrom
copilot/research-sgh-ensemble-generator-structure
Apr 1, 2026
Merged

Fix critical bugs in sgh_restart_ensemble test case#10
alexolinhager merged 2 commits into
sgh_ensemble_generator_templatefrom
copilot/research-sgh-ensemble-generator-structure

Conversation

Copy link
Copy Markdown

Copilot AI commented Apr 1, 2026

  • Understand compass config pattern (defaults live in shipped .cfg, read without fallback=)
  • In test_case.py configure(): add self.config.add_from_package(...) before reading any config options
  • Remove all fallback= kwargs from getint, getfloat, getboolean, and get calls in test_case.py
  • In ensemble_generator.cfg: change analysis_summary_file = REPLACE_WITH_... to analysis_summary_file = None (optional default; existing != 'none' guard handles it)
  • Run flake8: clean

Bug #1: config.get('restart_ensemble', {}) crashes because MpasConfigParser.get()
expects (section, option) positional args, not a dict fallback.
Fixed: config['restart_ensemble'] returns a SectionProxy with proper
.get()/.getint()/.getfloat()/.getboolean() methods.

Bug #2: _should_restart_run() looked for per-run analysis_results.json files
that are never written.  AnalysisStep writes analysis_summary.json to its
own work dir containing an individual_results dict for all runs.
Fixed: add analysis_summary_file config option; configure() loads the file
and passes per-run dicts to _should_restart_run() via a new run_results param.
RestartScheduler.create_config_file() now includes analysis_summary_file in
generated configs.

Bug #3: restart_attempt_N/ tracking dirs were never created by
InPlaceRestartMember.setup(), so max_consecutive_restarts was effectively
disabled and all attempt counters read 0.
Fixed: setup() now creates restart_attempt_N/ dirs using a single os.listdir()
call to find the highest existing attempt number.

Bug #5: restart_scheduler.py docstring Examples section referenced a
non-existent module path. Fixed to the correct path.

Agent-Logs-Url: https://github.com/alexolinhager/compass/sessions/ca2d29bf-1246-415c-bf2c-9de7521fa55f

Co-authored-by: alexolinhager <131483939+alexolinhager@users.noreply.github.com>
compass's MpasConfigParser does not support the fallback= keyword argument
on getint/getfloat/getboolean/get.  The correct compass pattern is to load
the shipped .cfg defaults via config.add_from_package() at the start of
configure() so that all options are resolvable without fallback= kwargs.

Changes:
- test_case.py: call self.config.add_from_package() at the top of configure()
  to register ensemble_generator.cfg as the source of defaults, then remove
  all fallback= kwargs from getint/getfloat/getboolean/get calls
- ensemble_generator.cfg: change analysis_summary_file from the
  REPLACE_WITH_... sentinel to `None` (the shipped default) so the existing
  `!= 'none'` guard correctly treats it as "not provided"

Agent-Logs-Url: https://github.com/alexolinhager/compass/sessions/219c1314-48f3-4507-852a-abf8e87511e4

Co-authored-by: alexolinhager <131483939+alexolinhager@users.noreply.github.com>
@alexolinhager alexolinhager marked this pull request as ready for review April 1, 2026 21:28
@alexolinhager alexolinhager merged commit 00cbd63 into sgh_ensemble_generator_template Apr 1, 2026
1 check passed
Copilot stopped work on behalf of alexolinhager due to an error April 1, 2026 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants