ULTRA l1a event_id update#3014
ULTRA l1a event_id update#3014lacoak21 wants to merge 1 commit intoIMAP-Science-Operations-Center:devfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates ULTRA processing to generate a new per-event event_id and wires it into event packet expansion, with accompanying unit tests. The PR also includes an unrelated change to ULTRA L2 rectangular map projection configuration.
Changes:
- Reworked ULTRA L0
get_event_idto produce per-event 50-character hex string IDs and integrated it intoprocess_ultra_events. - Updated/added unit tests for the new
get_event_idbehavior and its error path. - Changed ULTRA L2 rectangular map projection to only project
ena_intensity.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| imap_processing/ultra/l0/decom_ultra.py | Implements new string-based event_id generation and attaches IDs during event expansion. |
| imap_processing/ultra/l2/ultra_l2.py | Adjusts which variables are projected when producing RECTANGULAR L2 maps. |
| imap_processing/ultra/l1a/ultra_l1a.py | Adds commented-out raw packet note (currently unused). |
| imap_processing/tests/ultra/unit/test_ultra_l1a.py | Updates tests for the revised get_event_id function signature/output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # get raw packets for event data | ||
| # raw_packets = packet_generator(packet_file, xtce) |
There was a problem hiding this comment.
These commented-out lines referencing raw packets (raw_packets = ...) are leftover/debug code and add noise (and packet_generator isn’t imported here). Please remove them unless they’re needed for an imminent follow-up change.
| # get raw packets for event data | |
| # raw_packets = packet_generator(packet_file, xtce) |
| event_date = ( | ||
| b"\x929\xc4=\x05\x13\xf2dC\x0c`\x002\xb2\xb3\x80\n" | ||
| b'UQ\xb5BH\xe6\x114\x10O\t\xb1\x08\x0e`\x00\xd6\x89"\x00)UF\xd6' | ||
| ) | ||
| met = 445015657 | ||
| count = 2 | ||
| event_ids = get_event_id(event_date, count, met) |
There was a problem hiding this comment.
Minor typo: the variable name event_date appears to mean event_data in these tests. Renaming would improve clarity and avoid confusion when reading the test cases.
| binary = convert_to_binary_string(event_data) | ||
| # For all packets with event data, parses the binary string | ||
| bits_per_event = 166 | ||
| event_ids = [] |
There was a problem hiding this comment.
get_event_id hard-codes bits_per_event = 166, but ULTRA energy events use a different bit-length (see ENERGY_EVENT_FIELD_RANGES ending at bit 41 in ultra_utils.py). This will mis-parse or raise for energy event APIDs (897/961) when count > 0. Derive bits_per_event from the relevant field_ranges (or pass it in) so both event formats are supported, and add a unit test covering an energy-event packet.
| # Get the met value and convert to hex (4 bytes -> 8 hex ) | ||
| met_hex = format(shcoarse, "08x") | ||
| for i in range(count): | ||
| start_bit = i * bits_per_event | ||
| if start_bit + bits_per_event > len(binary): | ||
| raise ValueError( | ||
| f"Data for event {i} is expected to have more bits. Packet " | ||
| f"shcoarse={shcoarse}. Expected at least {start_bit + bits_per_event}" | ||
| f" bits, but got {len(binary)} bits." | ||
| ) | ||
| event_bits = binary[start_bit : start_bit + bits_per_event] | ||
| # Convert the event bits to an integer, then to a hex string, and concatenate | ||
| # with the met hex to create the event ID. | ||
| # Prepend "00" to the event bits to get an integral number of bytes | ||
| # (168 bits --> 21 bytes) | ||
| event_ids.append(met_hex + format(int("00" + event_bits, 2), "042x")) | ||
| return event_ids |
There was a problem hiding this comment.
This event ID generation does not incorporate APID or an event counter for repeated (APID, MET) pairs as described in issue #2823; it currently concatenates shcoarse with event payload bits. If the requirement is still APID+MET(+counter), update the implementation accordingly; otherwise the PR/issue text should be updated to match the new ID definition.
| def get_event_id(event_data: bytes, count: int, shcoarse: int) -> list: | ||
| """ | ||
| Get unique event IDs using data from events packets. | ||
| Generate unique event IDs for each event in the packet. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| shcoarse : numpy.ndarray | ||
| SHCOARSE (MET). | ||
| event_data : bytes | ||
| Raw event data from the packet. | ||
| count : int | ||
| Number of events in the packet. | ||
| shcoarse : np.ndarray | ||
| The met value for the packet. |
There was a problem hiding this comment.
The docstring/type hints for get_event_id are inconsistent (shcoarse is documented as np.ndarray but is used as a scalar int, and the return type is annotated as plain list). Please update the docstring and type hints to reflect the actual types (e.g., shcoarse: int / list[str]) to avoid confusing callers.
| event_ids: list[str] = [] | ||
| for i, count in enumerate(counts): | ||
| if count == 0: | ||
| all_events.append(empty_event) | ||
| all_indices.append(i) | ||
| event_ids.append("") # TODO ask ultra IT what to use here | ||
| else: |
There was a problem hiding this comment.
event_id is now populated with strings (and for count == 0 it uses ""), but ULTRA L1A CDF variable attributes currently define event_id as an int64 (see imap_processing/cdf/config/imap_ultra_l1a_variable_attrs.yaml). This mismatch is likely to break CDF writing/schema compliance. Update the CDF attrs and choose a consistent fill value for missing events (e.g., a fixed-length hex string) rather than leaving a TODO and variable-length IDs.
| rectangular_skymap, subdiv_depth_dict = healpix_skymap.to_rectangular_skymap( | ||
| rect_spacing_deg=output_map_structure.spacing_deg, | ||
| value_keys=healpix_skymap.data_1d.data_vars, | ||
| value_keys=["ena_intensity"], |
There was a problem hiding this comment.
Limiting value_keys to only "ena_intensity" will drop other rectangular-map variables such as ena_intensity_stat_uncert, but later code assumes map_dataset["ena_intensity_stat_uncert"] exists (used to create ena_intensity_sys_err). This will raise at runtime for RECTANGULAR output. Include all required variables in value_keys (at least intensity + stat_uncert) or adjust downstream logic accordingly.
| value_keys=["ena_intensity"], | |
| value_keys=["ena_intensity", "ena_intensity_stat_uncert"], |
Change Summary
closes #2823
Overview
Update the event id to contain the met AND the all the data for each event in a 50 character hex string
File changes
imap_processing/ultra/l0/decom_ultra.py
Testing
Tests for new event_id function
There will be a validation test in a future PR