Collapse romanisim step dir#17
Merged
Merged
Conversation
_05_romanisim.py selected its output directory from its flags, writing to 05_romanisim, 05_romanisim_l3, or 05_romanisim_l3_subpixel depending on --level and --dither-pattern. A step directory should name the step, not the flags it was invoked with; --level and --dither-pattern are run parameters like --max-systems. Every invocation now writes to 05_romanisim/. Drop the cores key script_05_romanisim_l3 in favor of script_05_romanisim for both levels, and remove _subpixel_pattern_names(), whose only caller was the directory branch. This also fixes --level l3 raising KeyError under simple.yaml and roman_test.yaml, which never carried the _l3 key. Point _06_h5_export.py's PREV_SCRIPT_NAME at 05_romanisim, matching what its own docstring already claimed. Variants of one config now overwrite each other, so --resume across differing --level/--dither-pattern is unsafe; the sentinels record neither. Expand the docstring caveat to say so. roman_data_challenge_rung_1_unlabeled.yaml had script_05_romanisim: 60 against _l3: 12; the collapsed key keeps 12 so L3 runs stay within their memory budget, lowering that config's L2 worker count. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mejiro_pipeline.py declares one --prev-step for _04, but _06_h5_export.py and calculate_snrs.py read the same attribute off the shared Namespace and so resolve their input to 03/ when driven through the orchestrator. Found while collapsing the romanisim step dirs; left unfixed as it is independent of that change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The romanisim section referenced mejiro.pipeline.romanisim_pipeline, removed when the romanisim scripts were consolidated into _05_romanisim, and a Step 1 section referenced _01_run_survey_simulation, removed when survey simulation was consolidated into _01b. Neither consolidation touched docs/, and Sphinx only warns on an unimportable automodule target, so both sections had been rendering as empty headings. Retarget the romanisim section to _05_romanisim and drop the redundant Step 1 section, since _01b already has one; also drop the now-misleading "(Pre-computed Tables)" qualifier from the 1b heading. Retargeting alone was not enough: conf.py mocks numpy but not asdf, so asdf's module-level numpy arithmetic raised TypeError against the mock and _05_romanisim still failed to import. Add asdf, roman_datamodels, romancal, and stcal to autodoc_mock_imports. This also restores mejiro_pipeline, which had been failing to import from the same gap. Two docutils warnings remain, from RST formatting in the _05_romanisim module docstring that only surfaced now that it renders. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Outputs-only re-execution; no cell sources changed. Picks up deflector_only True and the new main_halo_mass field from the regenerated export.
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.
This pull request updates documentation and configuration to reflect the consolidation of the L3 romanisim pipeline into a single step (
_05_romanisim.py), removing references to the previousromanisim_l3_pipeline.pyand clarifying directory and flag usage. It also improves Sphinx documentation support for new romanisim dependencies and corrects configuration and documentation references to the new pipeline structure.Documentation and pipeline step consolidation:
romanisim_l3_pipeline.pyto_05_romanisim.py, reflecting the consolidation of L3 romanisim processing into a single script and directory (05_romanisim/). This includes clarifying that all outputs, regardless of flags, are written to05_romanisim/, and that the resume logic now requires exact match of--leveland--dither-patternto avoid mixing incompatible products. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14]Sphinx and build system improvements:
asdf,roman_datamodels,romancal, andstcaltoautodoc_mock_importsinconf.pyto ensure Sphinx builds succeed even when these romanisim dependencies are not installed.Configuration cleanup:
script_05_romanisim_l3worker configuration fromscratch.yamlandtest.yaml, as this step is now merged intoscript_05_romanisim. [1] [2]Documentation bug report:
--prev-stepflag inmejiro_pipeline.py, outlining the problem, its effects, and suggested fixes.Minor documentation reorganization: