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

Update the titles and descriptions to those provided by INS #361

Merged

Conversation

WilliamJamieson
Copy link
Collaborator

This PR updates all the field/schema titles and descriptions to those listed by INS in https://innerspace.stsci.edu/display/RI/ASDF+L1+and+L2+metadata+keyword+review+assignments.

Checklist

@WilliamJamieson WilliamJamieson marked this pull request as ready for review January 23, 2024 18:57
Copy link

codecov bot commented Jan 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.38%. Comparing base (0fb1239) to head (16316b6).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #361   +/-   ##
=======================================
  Coverage   95.38%   95.38%           
=======================================
  Files           4        4           
  Lines         195      195           
=======================================
  Hits          186      186           
  Misses          9        9           

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

@WilliamJamieson
Copy link
Collaborator Author

Note, any field keyword changes, removals, or additions should be done in a separate PR as that type of change will be much more invasive to romancal. Please do make notes here and I will build an issue detailing those changes once this PR is finalized.

src/rad/resources/schemas/ramp-1.0.0.yaml Outdated Show resolved Hide resolved
src/rad/resources/schemas/ramp-1.0.0.yaml Outdated Show resolved Hide resolved
src/rad/resources/schemas/ramp-1.0.0.yaml Outdated Show resolved Hide resolved
title: Approximate CR magnitudes
title: Approximate Cosmic Ray Magnitudes (AB magnitude)
description: |
The magnitude of each segment that was flagged as having a cosmic ray hit.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't use this so it's not really important to talk about it. But as we haven't done the photometric calibration at this stage it is challenging for me to imagine that what is computed for Webb is in AB mags?

description: |
The Flat Data Array represents the small and large pixel-to-pixel
mitigations necessary to account for wavelength dependent detector
sensitivity and distortions in the optical path.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the most important piece of information here is the ~sign; is the flattened data raw * flat or raw / flat? Looking at the step, I know that it is raw / flat. Suggest adding:
An image with uniform sensitivity to a constant surface brightness scene can be obtained by dividing the raw data by the flat field.

Reference kernel used for convolving with data in order to correct
for interpixel capacitance
The kernel array used for convolving data to correct for interpixel
capacitance of neighboring pixels.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't actually do any correction here. Suggest:
The kernel by which a signal recorded by an idealized detector lacking IPC needs to be convolved in order to obtain the IPC-affected signal recorded by a realistic detector.

title: Nominal pixel area in steradians
title: Pixel Area (steradians)
description: |
The nominal pixel area in steradians.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does anyone know what the nominal pixel area is meant to represent? Is it a per SCA quantity or a per array quantity? Is it roughly the area of the central pixel or the average area of the pixels? @tddesjardins

title: Read Noise Data Array
description: |
The pixel-by-pixel map read noise data array is used in estimating the
expected noise in each pixel.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add (DN) ? Those are the units here.

title: Target RA (deg)
description: |
The right ascension (RA) of the target in degrees at the midpoint of the
exposure.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are things like the midpoint of the exposure relevant here? My impression is that this schema comes from the proposal time, before the midpoint of the exposure is known? @tddesjardins . There's also an "uncertainty" field below that's confusing to me conceptually; I don't think we're supposed to be doing astrometry on a particular source and feeding it back into the metadata, and instead are supposed to be ingesting a field from APT. Insofar as we do have an uncertainty in RA it would be good to know if that uncertainty is in "ra" or "ra * cos(dec)".

@WilliamJamieson WilliamJamieson force-pushed the feature/update_descriptions branch 4 times, most recently from 749a2d1 to c899099 Compare February 9, 2024 20:30
src/rad/resources/schemas/cal_step-1.0.0.yaml Outdated Show resolved Hide resolved
src/rad/resources/schemas/cal_step-1.0.0.yaml Outdated Show resolved Hide resolved
src/rad/resources/schemas/cal_step-1.0.0.yaml Outdated Show resolved Hide resolved
src/rad/resources/schemas/cal_step-1.0.0.yaml Outdated Show resolved Hide resolved
src/rad/resources/schemas/cal_step-1.0.0.yaml Outdated Show resolved Hide resolved
src/rad/resources/schemas/ramp-1.0.0.yaml Outdated Show resolved Hide resolved
src/rad/resources/schemas/ramp-1.0.0.yaml Outdated Show resolved Hide resolved
src/rad/resources/schemas/ramp-1.0.0.yaml Outdated Show resolved Hide resolved
src/rad/resources/schemas/ramp-1.0.0.yaml Outdated Show resolved Hide resolved
src/rad/resources/schemas/ramp-1.0.0.yaml Outdated Show resolved Hide resolved
@schlafly
Copy link
Collaborator

Since this ultimately just updates titles and descriptions, I'm just going to go ahead and approve it and we can take up further changes in subsequent PRs.

Copy link
Collaborator

@PaulHuwe PaulHuwe left a comment

Choose a reason for hiding this comment

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

Seconding @schlafly 's comment.

@PaulHuwe PaulHuwe merged commit 83dd329 into spacetelescope:main Feb 26, 2024
13 checks passed
tag: tag:stsci.edu:asdf/core/ndarray-1.*
datatype: uint32
exact_datatype: true
ndim: 2
dq_border_ref_pix_top:
title: DQ for border reference pixels, on top.
tag: tag:stsci.edu:asdf/core/ndarray-1.*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you mean to remove tag here?

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.

5 participants