Skip to content
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

Make sure logs get included in L3 products. #1207

Merged
merged 5 commits into from
May 2, 2024

Conversation

schlafly
Copy link
Collaborator

@schlafly schlafly commented May 1, 2024

This PR leads HLP products to have their "cal_logs" attribute filled out.

Previously these were not being populated because stpipe.core was explicitly checking that the output datamodel was an ImageModel. This PR changes that to check if it's either an ImageModel or a MosaicModel. It also clears the CalLogs when making a new output model to avoid some initial filler values.

Finally, a big portion of the HLP is processing the input L2s; flux calibration, sky matching, outlier detection. Those logs go into the L2 products which we do not save. This adds a new individual_image_cal_logs extension parallel to cal_logs that houses the individual L2 log messages. We probably want that?

Checklist

@schlafly schlafly added the HLPP label May 1, 2024
@schlafly schlafly added this to the 24Q3_B14 milestone May 1, 2024
@github-actions github-actions bot added the stpipe label May 1, 2024
Copy link

codecov bot commented May 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.03%. Comparing base (1898ca5) to head (3d7d787).
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1207      +/-   ##
==========================================
+ Coverage   76.78%   79.03%   +2.25%     
==========================================
  Files         113      116       +3     
  Lines        7933     7985      +52     
==========================================
+ Hits         6091     6311     +220     
+ Misses       1842     1674     -168     
Flag Coverage Δ *Carryforward flag
nightly 62.78% <ø> (?) Carriedforward from 47b29e2

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@schlafly schlafly marked this pull request as ready for review May 2, 2024 14:19
@schlafly schlafly requested a review from a team as a code owner May 2, 2024 14:19
@schlafly
Copy link
Collaborator Author

schlafly commented May 2, 2024

Regtests are "passing"; two new failures because of the added individual_image_cal_logs bit in the asdf file. If we like this approach, we should exclude those from compare_asdf.

@schlafly
Copy link
Collaborator Author

schlafly commented May 2, 2024

After adding individual_image_cal_logs to the list of extensions to ignore, regtests actually pass.
https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-Developers-Pull-Requests/726/

Copy link
Collaborator

@ddavis-stsci ddavis-stsci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks a bit messy but all the info appears to be copied.

@schlafly schlafly merged commit b5504d1 into spacetelescope:main May 2, 2024
30 checks passed
@schlafly schlafly deleted the include-hlp-logs branch May 2, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants