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

Known Issues to be resolved for version 1.2.0.0 - CHECKLIST #30

Open
5 of 12 tasks
acraugh opened this issue Jun 15, 2024 · 3 comments
Open
5 of 12 tasks

Known Issues to be resolved for version 1.2.0.0 - CHECKLIST #30

acraugh opened this issue Jun 15, 2024 · 3 comments

Comments

@acraugh
Copy link
Contributor

acraugh commented Jun 15, 2024

These are the known issues that need to be resolved for prior to release of version 1.2.0.0 of the NH mission dictionary.

LEISA Support

  • leisa_rate: This attribute has "rate" in its name, defines itself as a "frame rate", and gives a unit of measure in the description that is a unit of rate, but the defined unit of measure for the attribute is Units_of_Time. It sounds like the correct unit is 1/s, or Hz. If not, the definition needs to be rewritten. If an IM updates is needed, we can request one.
  • leisa_temperature: This attribute is allowed to be less than absolute zero. That seems bad. If this temperature is always in Kelvin (or Centigrade or Fahrenheit), then the unit should be constrained by Schematron rule and the appropriate lower limit set in the IngestLDD.

<Radiometric_Conversion_Constants>

  • Unless the Radiometric_Conversion_Constants class is relevant to all instruments, there should probably be a Schematron rule to validate that it is used if and only if it is applicable. On the other hand, if it's impossible to get it wrong, maybe not...
  • The Resolved_Source and Unresolved_Source classes really ought to be constrained to have the correct units, especially given the complexity and subtle difference in the two unit strings.
  • A regression test FAIL case should be added for the Resolved_Source and Unresolved_Source units constraint.

<Spacecraft_State>

  • pointing_method: This attribute (in Spacecraft_State) as currently defined is attempting to conflate two types of information into a single field, and there seems to be a significant piece of information missing. The defined values, first, are acronyms. Those terms need to be spelled out. Second, there are three related values beginning with "CB", which apparently stands for "Central Body" - but in this case the central body should be identified, especially if it might differ from the target identified in the Target_Area. Then, in the case of an (RA, Dec) pointing, there are values to be specified that should themselves be constrained and validated. This information needs to be provided in appropriate attributes a) so that users will understand it when they see it, and b) it can be validated and accessed programmatically without prior intimate knowledge of the data set.
  • gc_scan_rate: Whatever “gc” stands for should be spelled out. This attribute has no unit of measure but again seems like it should have a rate-appropriate unit to be understandable. Is it reasonable that the value is allowed to be negative?
    Note: This is being left as is. It will be revisited if it is raised as an issue in review.
  • Spacecraft_State cardinality: Currently this class is required, but is it possible to have data products which would use Observation_Parameters, yet have no applicable attributes in Spacecraft_State? If so, the class needs to be optional; if not, then a Schematron constraint should exist to ensure the class has some content, perhaps tied to instrument or some other attribute that will be present in the label.

General

  • The "Spectrograph" value currently included in detector_type should presumably be removed. The only spectrograph is, I think, Alice, and that has a detector_type of "Microchannel plate", right?
  • Should there be a Schematron rule to constrain what instrument classes are required in a label for each instrument/processing level? Might be desirable during migration, for example.
  • The documents reference the dictionary lookup tool page at https://pds.nasa.gov/datastandards/documents/dd/current/PDS4_PDS_DD_1L00/webhelp/all/. This page doesn't include the NH info as yet, and may never once the 1M00 version is created. This is still a manual process and has to be coordinated with Jordan and the link updated each time until EN can work out a more elegant solution.
  • CHECK and INCOMPLETE notations in the IngestLDD need to be resolved for met510 and target_motion_rate, respectively.
@benjhirsch
Copy link
Contributor

<Spacecraft_State> is a required class in <Observation_Parameters> but none of its attributes are required, which means it can be included empty. I know neither the LORRI nor Alice SBN templates include this class because it was added to the dictionary after we wrote the templates (@agicquelb correct if I'm wrong re: Alice). Can the class be made optional before release? If not, should we (a) include it empty (frowned upon) or (b) update our templates to fully include it?

@benjhirsch
Copy link
Contributor

Per conversation with @agicquelb, she did update the Alice template to account for the required class and added a few of the attributes. I'll do the same with LORRI for the sake of moving things along. Nevertheless, in the next update I feel that either <Spacecraft_State> should be optional or it should require a choice of at least one of its attributes.

@acraugh
Copy link
Contributor Author

acraugh commented Jun 24, 2024

Added an item under <Spacecraft_State> to revisit the cardinality and modify accordingly. Thanks!

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

No branches or pull requests

2 participants