fix: address SPDF follow-up for CoDICE, MAG, and SWAPI metadata#3307
Draft
davidt0x wants to merge 4 commits into
Draft
fix: address SPDF follow-up for CoDICE, MAG, and SWAPI metadata#3307davidt0x wants to merge 4 commits into
davidt0x wants to merge 4 commits into
Conversation
(A1.1) For "spin_sector" variable, could you change FORMAT to "I3" to cover FILLVAL=255 (A1.2) This is related to comment (3.2) below. Is it correct that "spin_angle" variable is defined as VAR_TYPE="support_data" and not VAR_TYPE="data". This seems to be the variable that shows what Spin Angle, and is not visible to users if VAR_TYPE="support_data". Implementation notes: - widened spin_sector FORMAT to I3 so FILLVAL=255 is representable. - promoted hi-sectored spin_angle to VAR_TYPE=data with DISPLAY_TYPE=spectrogram so it is visible to users. - broadcast the static spin-angle grid across epoch so the written CDF satisfies the ISTP DEPEND_0/1/2 requirements for a visible data variable.
(A2.1) "direction_label" variable values should be ["B_R", "B_T", "B_N"], or something like that, not ["Bx", "By", "Bz"], since RTN is a special case with named components. And same in imap_mag_l2_burst-rtn_20260422_v002.cdf (A3) For all imap_mag datasets, for "quality_bitmask" variable, FILLVAL attribute value has to be outside of [VALIDMIN, VALIDMAX] range. Now FILLVAL is same as VALIDMAX. Implementation notes: - RTN now writes ["B_R", "B_T", "B_N"], while SRF/DSRF/GSE/GSM keep ["Bx", "By", "Bz"] because those remain Cartesian component labels. - quality_bitmask now writes as CDF_UINT2 with VALIDMIN=0, VALIDMAX=255, and a non-science fill value outside that range. - the final fill value is 65535 rather than 256 because SPDF validation on the written CDF preferred the standard unsigned-16 fill value. - this change affects both MAG L2 and L1D because they share the same dataset-generation path.
(A4.1) Related to (12.2) below. I see that you now have "esa_energy" variable defined as VAR_TYPE="data", so visible to users. But that "esa_energy" variable (time varying 1-D, size 72) could also be used as DEPEND_1="esa_energy", instead of DEPEND_1="esa_step" (time non-varying 1-D, size 72). Depend variables in ISTP can be time-varying (1-D + time). Implementation notes: - swp_l1a_flags and the L2 rate/rate-uncertainty variables now use the time-varying esa_energy axis as DEPEND_1. - esa_energy itself remains indexed by esa_step because it is still the per-sweep mapping from step id to energy. - this was implemented in swapi_l2.py rather than by changing the shared SWAPI YAML defaults, because the shared defaults are also used by the L1 science writer and changing them there broke the L1 CDF path.
SPDF follow-up: Thanks, Dave. For (A1.2) comment, I missed that "spin_angle" variable is time non-varying, and cannot be easily changed to VAR_TYPE="data", so no change is needed in this case. Implementation notes: - restore hi-sectored spin_angle as a static 2-D support_data variable - remove the epoch broadcast and visible-data metadata that were added for the earlier interpretation of (A1.2) - keep the separate spin_sector FORMAT=I3 fix from (A1.1) - retain the focused test guard for spin_angle because the checked-in validation CDF still carries outdated spin_angle values
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.
Summary
This draft bundles the current in-situ SPDF follow-up changes for CoDICE, MAG, and SWAPI.
Final branch state:
quality_bitmaskmetadata so the fill value is out of range while preserving the0..255science range.esa_energyaxis.What Changed
CoDICE
spin_sectorFORMATtoI3soFILLVAL=255is representablespin_anglebecause the checked-in validation CDF still carries outdatedspin_anglevaluesMAG
direction_label = ["B_R", "B_T", "B_N"]Bx/By/Bzlabelsquality_bitmaskis written asCDF_UINT2withVALIDMIN=0,VALIDMAX=255, and an out-of-range fill valueSWAPI
DEPEND_1=esa_steptoDEPEND_1=esa_energyesa_energyitself indexed byesa_stepWhy
These changes address SPDF review comments on user-visible metadata and axis semantics while keeping the branch scoped to the specific issues raised in review.
Validation
pytest -q imap_processing/tests/codice/test_codice_hi_l2.py imap_processing/tests/mag/test_mag_l2.py imap_processing/tests/mag/test_mag_l1d.py imap_processing/tests/swapi/test_swapi_l2.py