Skip to content

GLOWS l1a validation corrections #2584

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.

Sign up for GitHub

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

Open
maxinelasp wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

GLOWS l1a validation corrections #2584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
12e0d6e
Validation corrections
maxinelasp Jan 15, 2026
fc26b75
Update DE processing to skip over empty packets
maxinelasp Jan 15, 2026
fe0e3cb
Set standard histogram size
maxinelasp Jan 15, 2026
93fd8d7
Fix tests
maxinelasp Jan 15, 2026
eae32a7
add new test, and fix docs
maxinelasp Jan 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions imap_processing/glows/l1a/glows_l1a.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from imap_processing.glows.l0.decom_glows import decom_packets
from imap_processing.glows.l0.glows_l0_data import DirectEventL0
from imap_processing.glows.l1a.glows_l1a_data import DirectEventL1A, HistogramL1A
from imap_processing.glows.utils.constants import GlowsConstants
from imap_processing.spice.time import (
met_to_ttj2000ns,
)
Expand Down Expand Up @@ -127,6 +128,9 @@ def generate_de_dataset(
"""
# TODO: Block header per second, or global attribute?

# Filter out DE records with no direct_events (incomplete packet sequences)
de_l1a_list = [de for de in de_l1a_list if de.direct_events is not None]
Comment on lines +131 to +132
Copy link
Contributor

@tech3371 tech3371 Jan 21, 2026

Choose a reason for hiding this comment

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

@greglucas, you just did that for CoDICE. I am wondering if we should refactor incomplete packets filter as general utils function.

Copy link
Collaborator

@greglucas greglucas Jan 21, 2026

Choose a reason for hiding this comment

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

I did add a warning for missing sequence counts for a given apid, but that is only when using the packet_file_to_datasets function which I don't think GLOWS is. But, I think that every instrument will want to handle these differently because some may want to keep it as a warning, others may have different conditions they want to consider on what a valid sequence of packets is.

What happens here if you have some direct_events but not all of the sequence, so not all of the data is missing and it is actually partially there. Maybe this is handled in creating the values in the first place.

Copy link
Contributor Author

@maxinelasp maxinelasp Jan 21, 2026

Choose a reason for hiding this comment

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

Yes, that case is handled later. The missing sequences are in an attribute for tracking. I did hear from Marek about some related changes surrounding duplicate DE packets, so this piece may get updated again soon.


# Store timestamps for each DirectEventL1a object.
time_data = np.zeros(len(de_l1a_list), dtype=np.int64)

Expand Down Expand Up @@ -292,10 +296,13 @@ def generate_histogram_dataset(
"""
# Store timestamps for each HistogramL1A object.
time_data = np.zeros(len(hist_l1a_list), dtype=np.int64)
# TODO Add daily average of histogram counts
# Data in lists, for each of the 25 time varying datapoints in HistogramL1A

hist_data = np.zeros((len(hist_l1a_list), 3600), dtype=np.uint16)
hist_data = np.full(
(len(hist_l1a_list), GlowsConstants.STANDARD_BIN_COUNT),
GlowsConstants.HISTOGRAM_FILLVAL,
dtype=np.uint16,
)

# First variable is the output data type, second is the list of values
support_data: dict = {
Expand Down Expand Up @@ -326,7 +333,9 @@ def generate_histogram_dataset(

for index, hist in enumerate(hist_l1a_list):
epoch_time = met_to_ttj2000ns(hist.imap_start_time.to_seconds())
hist_data[index] = hist.histogram
# Assign histogram data, padding with zeros if shorter than max_bins
hist_len = len(hist.histogram)
hist_data[index, :hist_len] = hist.histogram

support_data["flags_set_onboard"][1].append(hist.flags["flags_set_onboard"])
support_data["is_generated_on_ground"][1].append(
Expand All @@ -348,7 +357,8 @@ def generate_histogram_dataset(
dims=["epoch"],
attrs=glows_cdf_attributes.get_variable_attributes("epoch", check_schema=False),
)
bin_count = 3600 # TODO: Is it always 3600 bins?

bin_count = GlowsConstants.STANDARD_BIN_COUNT

bins = xr.DataArray(
np.arange(bin_count),
Expand Down
16 changes: 14 additions & 2 deletions imap_processing/glows/l1a/glows_l1a_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
"""Data classes to support GLOWS L1A processing."""

import logging
import struct
from dataclasses import InitVar, dataclass, field

from imap_processing.glows import __version__
from imap_processing.glows.l0.glows_l0_data import DirectEventL0, HistogramL0
from imap_processing.glows.utils.constants import DirectEvent, TimeTuple

logger = logging.getLogger(__name__)


@dataclass
class StatusData:
Expand Down Expand Up @@ -261,8 +264,7 @@ def __post_init__(self, l0: HistogramL0) -> None:
self.glows_time_offset = TimeTuple(l0.GLXOFFSEC, l0.GLXOFFSUBSEC)

# In L1a, these are left as unit encoded values.
# TODO: This is plus one in validation code, why?
self.number_of_spins_per_block = l0.SPINS + 1
self.number_of_spins_per_block = l0.SPINS
self.number_of_bins_per_histogram = l0.NBINS
self.number_of_events = l0.EVENTS
self.filter_temperature_average = l0.TEMPAVG
Expand All @@ -280,6 +282,16 @@ def __post_init__(self, l0: HistogramL0) -> None:
"is_generated_on_ground": False,
}

# Remove the extra byte from some packets (if there are an odd number of bins)
if self.number_of_bins_per_histogram % 2 == 1:
self.histogram = self.histogram[:-1]

if self.number_of_bins_per_histogram != len(self.histogram):
logger.warning(
f"Number of bins {self.number_of_bins_per_histogram} does not match "
f"processed number of bins {len(self.histogram)}!"
)
Comment on lines +286 to +293
Copy link
Contributor

@tech3371 tech3371 Jan 21, 2026

Choose a reason for hiding this comment

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

so this is trimming self.histogram variable array and then comparing. Looks like both are if statement and not elif. Is that expected?

Copy link
Contributor Author

@maxinelasp maxinelasp Jan 21, 2026

Choose a reason for hiding this comment

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

Yes, I only want to trim one byte off the end, because that's the only valid case. Then, I check if the length mismatches - if it does, that means I either have much less data than expected, or more, which are both error cases.

Copy link
Collaborator

@greglucas greglucas Jan 21, 2026

Choose a reason for hiding this comment

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

Do you want to raise or logger.error it instead of a warning then?

Does this mean that the flight software is always transmitting an even number of bytes then so you are taking care of that above? That seems somewhat odd to me and I'm not sure why they would send an extra byte down.

Copy link
Contributor Author

@maxinelasp maxinelasp Jan 21, 2026

Choose a reason for hiding this comment

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

Yes, the flight software always transmits an even number of bytes so if there is an odd number of bins, we end up with an extra zero at the end. This is something that came up in MAG too so I think this is what is occurring.

Copy link
Contributor Author

@maxinelasp maxinelasp Jan 21, 2026

Choose a reason for hiding this comment

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

I can change it to raise an error

Copy link
Collaborator

@greglucas greglucas Jan 21, 2026

Choose a reason for hiding this comment

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

Up to you what you want to do. There are pros and cons. It does fail loudly, but that also means it fails loudly and other people get upset with us for not producing the good data that was there 😂

Copy link
Contributor Author

@maxinelasp maxinelasp Jan 21, 2026

Choose a reason for hiding this comment

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

Yeah, I think I would rather this just be evident if I am looking. If the data is as expected, then does it really matter...? I don't really use number of bins in the later code, it's all based off the histogram lengths anyway.



@dataclass
class DirectEventL1A:
Expand Down
12 changes: 8 additions & 4 deletions imap_processing/glows/packet_definitions/GLX_COMBINED.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,14 @@ P_GLX_TMSCDE.xml and P_GLX_TMSHIST.xml to process both packet APIDs -->
<xtce:IntegerParameterType name="INT32" signed="true">
<xtce:IntegerDataEncoding sizeInBits="32" encoding="signed" />
</xtce:IntegerParameterType>
<xtce:BinaryParameterType name="BYTE28800">
<xtce:BinaryParameterType name="BYTEHIST">
<xtce:BinaryDataEncoding bitOrder="mostSignificantBitFirst">
<xtce:SizeInBits>
<xtce:FixedValue>28800</xtce:FixedValue>
<xtce:DynamicValue>
<xtce:ParameterInstanceRef parameterRef="PKT_LEN"/>
<!--To find the rest of the packet, this intercept is -(32 + 32 + 16 + 16 + 24 + 32 + 24 + 16 + 24 + 32 + 24 + 16 + 24 + 8 + 16 + 8 + 16 + 16 + 32 + 16 + 32 + 8 + 16 + 32) = 512 bits for all HIST fields before HISTOGRAM_DATA. Then +8 because PKT_LEN is zero indexed, resulting in an offset of -504. -->
<xtce:LinearAdjustment intercept="-504" slope="8"/>
</xtce:DynamicValue>
</xtce:SizeInBits>
</xtce:BinaryDataEncoding>
</xtce:BinaryParameterType>
Expand Down Expand Up @@ -172,9 +176,9 @@ P_GLX_TMSCDE.xml and P_GLX_TMSHIST.xml to process both packet APIDs -->
<xtce:ShortDescription>Number of events</xtce:ShortDescription>
<xtce:LongDescription>Number of event in all bins (sum) of this histogram.</xtce:LongDescription>
</xtce:Parameter>
<xtce:Parameter name="HISTOGRAM_DATA" parameterTypeRef="BYTE28800">
<xtce:Parameter name="HISTOGRAM_DATA" parameterTypeRef="BYTEHIST">
<xtce:ShortDescription>Histogram Counts</xtce:ShortDescription>
<xtce:LongDescription>Total histogram data counts. Each bin has 8 bits of data, with 3600 total bins.</xtce:LongDescription>
<xtce:LongDescription>Total histogram data counts. Each bin has 8 bits of data. Number of bins is dynamically determined from packet length.</xtce:LongDescription>
</xtce:Parameter>
<!-- Direct event parameters -->
<xtce:Parameter name="LEN" parameterTypeRef="UINT16">
Expand Down
6 changes: 6 additions & 0 deletions imap_processing/glows/utils/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,16 @@ class GlowsConstants:
IMAP clock)
SCAN_CIRCLE_ANGULAR_RADIUS: float
angular radius of IMAP/GLOWS scanning circle [deg]
HISTOGRAM_FILLVAL: int
Fill value for histogram bins (65535 for uint16)
STANDARD_BIN_COUNT: int
Standard number of bins per histogram (3600)
"""

SUBSECOND_LIMIT: int = 2_000_000
SCAN_CIRCLE_ANGULAR_RADIUS: float = 75.0
HISTOGRAM_FILLVAL: int = 65535
STANDARD_BIN_COUNT: int = 3600


@dataclass
Expand Down
46 changes: 45 additions & 1 deletion imap_processing/tests/glows/test_glows_l1a_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,45 @@ def test_histogram_list(histogram_test_data, decom_test_data):
assert sum(histogram_test_data.histogram) == histl0.EVENTS


def test_histogram_bin_handling(decom_test_data):
"""Test histogram bin handling for odd bins."""
histl0 = decom_test_data[0][0]

# Test odd number of bins - extra byte should be removed
mock_histl0_odd = mock.MagicMock(spec=histl0)
mock_histl0_odd.NBINS = 3599
mock_histl0_odd.HISTOGRAM_DATA = list(range(3600))
mock_histl0_odd.SWVER = 1
mock_histl0_odd.packet_file_name = "test.pkts"
mock_histl0_odd.ccsds_header = mock.MagicMock()
mock_histl0_odd.ccsds_header.SRC_SEQ_CTR = 0
mock_histl0_odd.STARTID = 0
mock_histl0_odd.ENDID = 10
mock_histl0_odd.SEC = 1000
mock_histl0_odd.SUBSEC = 0
mock_histl0_odd.OFFSETSEC = 0
mock_histl0_odd.OFFSETSUBSEC = 0
mock_histl0_odd.GLXSEC = 1000
mock_histl0_odd.GLXSUBSEC = 0
mock_histl0_odd.GLXOFFSEC = 0
mock_histl0_odd.GLXOFFSUBSEC = 0
mock_histl0_odd.SPINS = 10
mock_histl0_odd.EVENTS = 1000
mock_histl0_odd.TEMPAVG = 100
mock_histl0_odd.TEMPVAR = 10
mock_histl0_odd.HVAVG = 500
mock_histl0_odd.HVVAR = 5
mock_histl0_odd.SPAVG = 150
mock_histl0_odd.SPVAR = 1
mock_histl0_odd.ELAVG = 20
mock_histl0_odd.ELVAR = 2
mock_histl0_odd.FLAGS = 0

hist_odd = HistogramL1A(mock_histl0_odd)
assert len(hist_odd.histogram) == 3599
assert hist_odd.number_of_bins_per_histogram == 3599


def test_histogram_obs_day(packet_path):
l1a = glows_l1a(packet_path)

Expand Down Expand Up @@ -522,10 +561,11 @@ def test_expected_hist_results(l1a_dataset):
}

# block header and flags are handled differently, so not tested here
# "number_of_spins_per_block" is a special case and handled specifically
# (validation data is incorrect)
Copy link
Contributor

@tech3371 tech3371 Jan 20, 2026

Choose a reason for hiding this comment

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

Is there plan to fix that mismatch from validation data?

Copy link
Contributor Author

@maxinelasp maxinelasp Jan 21, 2026

Choose a reason for hiding this comment

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

I can look into it. I don't think it's worth putting tons of time in because this is a pretty minor mismatch, but I can try and re-run the validation data through the new version of the validation code and see if that produces the new output.

compare_fields = [
"first_spin_id",
"last_spin_id",
"number_of_spins_per_block",
"number_of_bins_per_histogram",
"histogram",
"number_of_events",
Expand Down Expand Up @@ -560,6 +600,10 @@ def test_expected_hist_results(l1a_dataset):

for field in compare_fields:
assert np.array_equal(data[field], datapoint[field].data)
assert np.array_equal(
data["number_of_spins_per_block"] - 1,
datapoint["number_of_spins_per_block"].data,
)


@mock.patch("imap_processing.glows.l1a.glows_l1a.decom_packets")
Expand Down
1 change: 0 additions & 1 deletion imap_processing/tests/glows/test_glows_l1b_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ def test_validation_data_histogram(
"glows_end_time_offset": "glows_time_offset",
"imap_start_time": "imap_start_time",
"imap_end_time_offset": "imap_time_offset",
"number_of_spins_per_block": "number_of_spins_per_block",
"number_of_bins_per_histogram": "number_of_bins_per_histogram",
"histogram": "histogram",
"number_of_events": "number_of_events",
Expand Down