-
Notifications
You must be signed in to change notification settings - Fork 28
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
Switch exposure pipeline to use ModelLibrary instead of a list of models #1525
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1525 +/- ##
==========================================
+ Coverage 76.68% 77.00% +0.31%
==========================================
Files 120 120
Lines 7832 7796 -36
==========================================
- Hits 6006 6003 -3
+ Misses 1826 1793 -33 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
@@ -300,5 +300,5 @@ def test_pipeline_suffix(rtdata, ignore_asdf_paths): | |||
assert model.meta.cal_step.flat_field == "COMPLETE" | |||
assert model.meta.cal_step.photom == "COMPLETE" | |||
assert model.meta.cal_step.source_detection == "COMPLETE" | |||
assert model.meta.cal_step.tweakreg == "INCOMPLETE" | |||
assert model.meta.cal_step.tweakreg == "SKIPPED" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assert
isn't currently reached in the regtests due to the compare_against_truth
failing:
FAILED romancal/regtest/test_wfi_image_pipeline.py::test_pipeline_suffix - AssertionError:
Diff report for:
result file: /runner/_work/_temp/pytest_basetemp/popen-gw2/test_pipeline_suffix0/r0000101001001001001_0001_wfi01_star.asdf
model type: ImageModel
truth file: /runner/_work/_temp/pytest_basetemp/popen-gw2/test_pipeline_suffix0/truth/r0000101001001001001_0001_wfi01_star.asdf
model type: ImageModel
{'dictionary_item_added': ["root['roman']['meta']['asn']", "root['roman']['meta']['exptype']", "root['roman']['meta']['group_id']"],
'values_changed': {"root['roman']['meta']['cal_step']['tweakreg']": {'new_value': 'SKIPPED',
'old_value': 'INCOMPLETE'}}}
But when 'okified' this change to the assert will be needed.
Thanks. This looks good. You also deleted some orphaned fully saturated exposure handling here. I think following your investigation we probably do want to keep and start using that code again, rather than deleting it? @ddavis-stsci , this refactors some of the pipeline handling to use ModelLibrary more uniformly, so you may want to weigh in here. |
Converted to draft to attempt to also address #1523 and some other elp cleanup. |
This PR updates the exposure pipeline to use a
ModelLibrary
instead of a list of models. This allows:Regtests: https://github.com/spacetelescope/RegressionTests/actions/runs/11861738024
show 2 expected failures:
The first is a byproduct of using ModelLibrary. The output files for elp now contain asn information.
The second shows that
meta.cal_step.tweakreg
is being correctly marked as skipped for test_pipeline_suffix.Tasks
24Q4_B15
(use the latest build if not sure)no-changelog-entry-needed
)changes/
:echo "changed something" > changes/<PR#>.<changetype>.rst
(see below for change types)docs/
pageokify_regtests
to update the truth filesnews fragment change types...
changes/<PR#>.general.rst
: infrastructure or miscellaneous changechanges/<PR#>.docs.rst
changes/<PR#>.stpipe.rst
changes/<PR#>.associations.rst
changes/<PR#>.scripts.rst
changes/<PR#>.mosaic_pipeline.rst
changes/<PR#>.patch_match.rst
steps
changes/<PR#>.dq_init.rst
changes/<PR#>.saturation.rst
changes/<PR#>.refpix.rst
changes/<PR#>.linearity.rst
changes/<PR#>.dark_current.rst
changes/<PR#>.jump_detection.rst
changes/<PR#>.ramp_fitting.rst
changes/<PR#>.assign_wcs.rst
changes/<PR#>.flatfield.rst
changes/<PR#>.photom.rst
changes/<PR#>.flux.rst
changes/<PR#>.source_detection.rst
changes/<PR#>.tweakreg.rst
changes/<PR#>.skymatch.rst
changes/<PR#>.outlier_detection.rst
changes/<PR#>.resample.rst
changes/<PR#>.source_catalog.rst