Feat/update mag frame #2497

mhairifin · 2025-12-10T12:02:34Z

Change Summary

Overview

Update MAGO and MAGI spice frames to use IMAP_MAG_BASE instead of the mechanical spice frames. The MAG team have agreed to use an ideal frame for the spice transformation as our in flight calibration already includes rotation into the idealised frame.
Update MAG L2 and L1D variable names for the vectors

Updated Files

ialirt/l0/parse_mag
- Change MAGO and MAGI frame to IMAP_MAG_BASE for I-ALiRT
mag/l2/mag_l2_data
- Introduce additional fields to ValidFrames enum to distinguish MAGI and MAGO despite sharing a spice frame, and add in the variable name and variable attributes identifier
- Use these new fields to generate an L2 dataset
mag/l1d/mag_l1d_data
- Use new spice_frame formulation

Testing

Tests updated where needed to use spice_frame
Frame rotation test updated to reflect that MAGI and MAGO share a spice frame
L2 dataset generation tests updated to test for correct variable name

…adjust for MAGO and MAGI having the same spice frame

… and MAGI might share a spice frame

…e is in dataset

alastairtree

Looks very tidy to me!

alastairtree · 2025-12-10T12:16:44Z

imap_processing/mag/l2/mag_l2_data.py

-    GSM = SpiceFrame.IMAP_GSM
-    RTN = SpiceFrame.IMAP_RTN
+    MAGO = ("MAGO", SpiceFrame.IMAP_MAG_BASE, "vector_attrs", "vectors")
+    MAGI = ("MAGi", SpiceFrame.IMAP_MAG_BASE, "vector_attrs", "vectors")


If both sensors are in the same spice frame IMAP_MAG_BASE what are the consequences? I had assumed we needed both to be different to be able to handle the the slightly different positions of them

We do need to transform them differently - but we already do so by providing separate calibration matrices which are used for L2 and L1D. We create those calibration matrices by using the magnetic field data to work out how non-aligned with the spacecraft reference frame the sensors are. So either we use an idealised transform here, or we would have to create a calibration matrix to align it, and then un-twist it by this spice transform before we provide it as ancillary data.

Based on our chat we have agreed

Fix the lowercase MAGi=>MAGI

Add comment explain why BASE frame is used for L2/L1D only

Add frames for the unused, but still valid, SPICE magi/o frames and call them "ground calibration"

…s change

maxinelasp

Looks good! Nice improvement. I think we should hold back on merging this until I'm able to investigate L3 MAG usage, but I can deploy this to dev now for further testing. I poked around more and it doesn't look like anywhere in the MAG codebase assumes that the main variable is named "vectors".

maxinelasp · 2025-12-11T16:42:42Z

imap_processing/ialirt/l0/parse_mag.py

            time_data["secondary_epoch"],
            magi_out,
-            SpiceFrame.IMAP_MAG_I,
+            SpiceFrame.IMAP_MAG_BASE,


I think it would make sense to point this to ValidFrames.IMAP_MAGI so it can benefit from that abstraction layer if ValidFrames gets updated again.

maxinelasp · 2025-12-11T16:52:59Z

imap_processing/mag/l2/mag_l2_data.py

+    DSRF = ("DSRF", SpiceFrame.IMAP_DPS, "vector_attrs_dsrf", "b_dsrf")
+    SRF = ("SRF", SpiceFrame.IMAP_SPACECRAFT, "vector_attrs_srf", "b_srf")
+    GSE = ("GSE", SpiceFrame.IMAP_GSE, "vector_attrs_gse", "b_gse")
+    GSM = ("GSM", SpiceFrame.IMAP_GSM, "vector_attrs_gsm", "b_gsm")


There are a few people who are using MAG data as inputs - we will need to reach out to them if we change variable names here to make sure they can access the correct variable. (Or open a PR against L3 code, I can take a look at this.)

The current users of MAG are:
CoDICE L3b
HIT L3
SWE L3

That's not too many, so I think it's fine to change the names - just something we should consider and check.

ping @jtniehof and @pleasant-menlo to see if this renaming of MAG frames impacts you.

Just to confirm, this will not change the "vectors" variable name in the Mag L1d products? If not, this does not cause any issues for L3 processing.

Thanks! - Ethan and Peter

Hi - it does change the "vectors" variable name. It will be b_gse/b_dsrf/b_srf etc. dependent on the utilised reference frame

@mhairifin do you want all of these variables in the same file now? i.e. we'll have one l2_norm file with all 3 of those vectors now, or will the filename be l2_norm-dsrf AND have a b_dsrf variable in it?

We are happy with each reference frame in a different file - we would just like to change the variable name to use one that is more standard for magnetic fields, and to specify the co-ordinate system in the variable name for consistency with I-ALiRT and more clarity for science end users

OK, I have a slight concern that this should have all been discussed long ago where @maxinelasp and Drew led a discussion at one of the science team meetings several years ago now I think to align all instruments on the naming conventions and settle on a consistent naming for variables. I personally prefer this update (I didn't like vectors as being too generic), but just want to put this down on asking why this is happening so late in the game here. Maybe it was just never updated by the SDC and this is what was settled on long ago.

I believe we did discuss changing the names of these variables over the summer with @maxinelasp but it never became the highest priority bit of work, and I don't think we were aware it would be a problem. I don't think it's an unreasonable concern, and if you consider it a blocking issue then we can discuss internally how to move forwards, but these variable names would be our preference.

Hi, thanks for the clarification that it will change variable names. We created a branch in L3 that will use the new variable name if/when this change goes through. We will have to redeploy SWE, HIT, and CoDICE to prod when the L2 change gets deployed.

Thanks,
Patrick and Sebastian

greglucas · 2025-12-18T16:18:17Z

@mhairifin it looks like you have conflicts from the last merge now that need to be resolved.

greglucas

Lets get this in then if you all want it and L3 has been updated fairly easily. @pleasant-menlo feel free to make the changes then and we can deploy new containers all at the same time with this update and yours.

pleasant-menlo · 2025-12-18T19:48:22Z

@greglucas We deployed the SWE, Hit, and CoDICE containers to dev with this change, they are ready to be deployed into production.

CAVA expects variable names to be consistent across a whole dataset. Is it possible or planned to reprocess all of the mag data that has been produced so far in production after this change?

Thanks,
Patrick and Sebastian

greglucas · 2025-12-18T19:54:45Z

CAVA expects variable names to be consistent across a whole dataset. Is it possible or planned to reprocess all of the mag data that has been produced so far in production after this change?

Absolutely! This is an SPDF archiving requirement too I believe.

greglucas · 2025-12-22T20:40:16Z

I have deployed this update in the latest MAG container release, and I have pushed the SWE, HIT, and CoDICE L3 containers to production to align with that update. Let me know if there are any other updates needed.

@maxinelasp is going to work on reprocessing the l1d MAG files later today/tomorrow after the pointing attitude jobs get reprocessed with recent updates from them that would be good to have in the MAG files.

mhairifin added 6 commits December 9, 2025 17:19

Change MAGo and MAGi frame to MAG BASE frame for I-ALiRT, L1D, and L2

7dd136f

Add more parameters to valid frames enum to capture new names and to …

03d7566

…adjust for MAGO and MAGI having the same spice frame

Update test to use new spice_frame formulation, and reflect that MAGO…

18301b5

… and MAGI might share a spice frame

Update tests to use spice_frame, and assert that correct variable nam…

b710ef9

…e is in dataset

Restore this line as it no longer needs to be changed

cc2ee66

fix: typo

4cbc455

alastairtree reviewed Dec 10, 2025

View reviewed changes

mhairifin added 4 commits December 10, 2025 13:49

pre-commit fixes

5deff0b

Address comments from discussion with Alastair on future proofing thi…

ec7ea30

…s change

Update tests to use new variable names

1a82711

fix: issues identified by precommit

1ec703d

mhairifin marked this pull request as ready for review December 10, 2025 14:58

mhairifin requested a review from alastairtree December 10, 2025 14:58

maxinelasp self-requested a review December 11, 2025 16:39

maxinelasp assigned maxinelasp and mhairifin and unassigned maxinelasp Dec 11, 2025

maxinelasp reviewed Dec 11, 2025

View reviewed changes

Use spice frame abstraction in I-ALiRT

707cfa1

merge and resolve conflicts

cf26caa

alastairtree approved these changes Dec 18, 2025

View reviewed changes

greglucas approved these changes Dec 18, 2025

View reviewed changes

greglucas merged commit e98b36c into IMAP-Science-Operations-Center:dev Dec 18, 2025
14 checks passed

Feat/update mag frame #2497

Feat/update mag frame #2497

Uh oh!

Conversation

mhairifin commented Dec 10, 2025

Change Summary

Overview

Updated Files

Testing

Uh oh!

alastairtree left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxinelasp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greglucas commented Dec 18, 2025

Uh oh!

greglucas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pleasant-menlo commented Dec 18, 2025

Uh oh!

greglucas commented Dec 18, 2025

Uh oh!

greglucas commented Dec 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants