From daad19917c7dedeb17a2fb1e21b0f3123439c52e Mon Sep 17 00:00:00 2001 From: Tim Plummer Date: Wed, 11 Mar 2026 09:24:29 -0600 Subject: [PATCH 1/3] Add diagfee product to hi goodtimes upstream dependencies --- imap_processing/cli.py | 12 +++++++++++- imap_processing/tests/test_cli.py | 27 ++++++++------------------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/imap_processing/cli.py b/imap_processing/cli.py index bf1501f50..d36453dd8 100644 --- a/imap_processing/cli.py +++ b/imap_processing/cli.py @@ -832,14 +832,24 @@ def do_processing( # noqa: PLR0912 f"got {len(cal_prod_paths)}" ) + l1a_diagfee_paths = dependencies.get_file_paths( + source="hi", data_type="l1a", descriptor="diagfee" + ) + if len(l1a_diagfee_paths) != 1: + raise ValueError( + f"Expected one L1A DIAG_FEE file, got {len(l1a_diagfee_paths)}" + ) + # Load CDFs before passing to hi_goodtimes l1b_de_datasets = [load_cdf(path) for path in l1b_de_paths] l1b_hk = load_cdf(l1b_hk_paths[0]) + l1a_diagfee = load_cdf(l1a_diagfee_paths[0]) datasets = hi_goodtimes.hi_goodtimes( - l1b_de_datasets, self.repointing, + l1b_de_datasets, l1b_hk, + l1a_diagfee, cal_prod_paths[0], ) else: diff --git a/imap_processing/tests/test_cli.py b/imap_processing/tests/test_cli.py index 0bbbe1ecd..774b306f7 100644 --- a/imap_processing/tests/test_cli.py +++ b/imap_processing/tests/test_cli.py @@ -373,21 +373,8 @@ def test_hi_l1b_goodtimes(mock_hi_goodtimes, mock_instrument_dependencies): mock_hi_goodtimes.return_value = [mock_goodtimes_ds] mocks["mock_write_cdf"].return_value = Path("/path/to/goodtimes_output.cdf") - # Mock load_cdf to return xr.Dataset objects - mock_de_dataset = xr.Dataset() - mock_hk_dataset = xr.Dataset() - # 7 DE files + 1 HK file = 8 total calls to load_cdf - mocks["mock_load_cdf"].side_effect = [ - mock_de_dataset, - mock_de_dataset, - mock_de_dataset, - mock_de_dataset, - mock_de_dataset, - mock_de_dataset, - mock_de_dataset, - mock_hk_dataset, - ] - + # set load_cdf to return empty datasets + mocks["mock_load_cdf"].return_value = xr.Dataset() # Set up the input collection with required dependencies input_collection = ProcessingInputCollection( ScienceInput("imap_hi_l1b_45sensor-de_20250415-repoint00001_v001.cdf"), @@ -398,6 +385,7 @@ def test_hi_l1b_goodtimes(mock_hi_goodtimes, mock_instrument_dependencies): ScienceInput("imap_hi_l1b_45sensor-de_20250415-repoint00006_v001.cdf"), ScienceInput("imap_hi_l1b_45sensor-de_20250415-repoint00007_v001.cdf"), ScienceInput("imap_hi_l1b_45sensor-hk_20250415-repoint00004_v001.cdf"), + ScienceInput("imap_hi_l1a_45sensor-diagfee_20250415-repoint00004_v001.cdf"), AncillaryInput("imap_hi_45sensor-cal-prod_20240101_v001.csv"), ) mocks["mock_pre_processing"].return_value = input_collection @@ -416,17 +404,18 @@ def test_hi_l1b_goodtimes(mock_hi_goodtimes, mock_instrument_dependencies): instrument.process() # Verify load_cdf was called for DE files and HK file - assert mocks["mock_load_cdf"].call_count == 8 # 7 DE + 1 HK + assert mocks["mock_load_cdf"].call_count == 9 # 7 DE + 1 HK + 1 DIAG_FEE # Verify hi_goodtimes was called with correct arguments assert mock_hi_goodtimes.call_count == 1 call_args = mock_hi_goodtimes.call_args # Check that datasets (not paths) were passed for l1b_de_datasets and l1b_hk - assert isinstance(call_args.args[0], list) # l1b_de_datasets is a list - assert len(call_args.args[0]) == 7 # 7 DE datasets + assert call_args.args[0] == "repoint00004" # current_repointing + assert isinstance(call_args.args[1], list) # l1b_de_datasets is a list + assert len(call_args.args[1]) == 7 # 7 DE datasets assert isinstance(call_args.args[2], xr.Dataset) # l1b_hk is a dataset - assert call_args.args[1] == "repoint00004" # current_repointing + assert isinstance(call_args.args[3], xr.Dataset) # l1a_diagfee is a dataset # goodtimes now returns xr.Dataset, so write_cdf should be called assert mocks["mock_write_cdf"].call_count == 1 From b315fc26f82417a9e19cc3ed8877c3e3d9daadc0 Mon Sep 17 00:00:00 2001 From: Tim Plummer Date: Wed, 11 Mar 2026 09:30:22 -0600 Subject: [PATCH 2/3] Add mark_bad_tdc_cal algorithm to hi_goodtimes --- imap_processing/hi/hi_goodtimes.py | 122 +++++++- imap_processing/tests/hi/test_hi_goodtimes.py | 283 +++++++++++++++++- 2 files changed, 383 insertions(+), 22 deletions(-) diff --git a/imap_processing/hi/hi_goodtimes.py b/imap_processing/hi/hi_goodtimes.py index f29a90b42..0d2347abd 100644 --- a/imap_processing/hi/hi_goodtimes.py +++ b/imap_processing/hi/hi_goodtimes.py @@ -45,9 +45,10 @@ class CullCode(IntEnum): def hi_goodtimes( - l1b_de_datasets: list[xr.Dataset], current_repointing: str, + l1b_de_datasets: list[xr.Dataset], l1b_hk: xr.Dataset, + l1a_diagfee: xr.Dataset, cal_product_config_path: Path, ) -> list[xr.Dataset]: """ @@ -58,23 +59,26 @@ def hi_goodtimes( 1. mark_incomplete_spin_sets - Remove incomplete 8-spin histogram periods 2. mark_drf_times - Remove times during spacecraft drift restabilization - 3. mark_overflow_packets - Remove times when DE packets overflow - 4. mark_statistical_filter_0 - Detect drastic penetrating background changes - 5. mark_statistical_filter_1 - Detect isotropic count rate increases - 6. mark_statistical_filter_2 - Detect short-lived event pulses + 3. mark_bad_tdc_cal - Remove times with failed TDC calibration + 4. mark_overflow_packets - Remove times when DE packets overflow + 5. mark_statistical_filter_0 - Detect drastic penetrating background changes + 6. mark_statistical_filter_1 - Detect isotropic count rate increases + 7. mark_statistical_filter_2 - Detect short-lived event pulses Parameters ---------- + current_repointing : str + Repointing identifier for the current pointing (e.g., "repoint00001"). + Used to identify which dataset in l1b_de_datasets is the current one. l1b_de_datasets : list[xr.Dataset] L1B DE datasets for surrounding pointings. Typically includes current plus 3 preceding and 3 following pointings (7 total). Statistical filters 0 and 1 use all datasets; other filters use only the current pointing. - current_repointing : str - Repointing identifier for the current pointing (e.g., "repoint00001"). - Used to identify which dataset in l1b_de_datasets is the current one. l1b_hk : xr.Dataset L1B housekeeping dataset containing DRF status. + l1a_diagfee : xr.Dataset + L1A DIAG_FEE dataset containing TDC calibration status. cal_product_config_path : Path Path to calibration product configuration CSV file. @@ -131,6 +135,7 @@ def hi_goodtimes( l1b_de_datasets, current_index, l1b_hk, + l1a_diagfee, cal_product_config_path, ) else: @@ -200,6 +205,7 @@ def _apply_goodtimes_filters( l1b_de_datasets: list[xr.Dataset], current_index: int, l1b_hk: xr.Dataset, + l1a_diagfee: xr.Dataset, cal_product_config_path: Path, ) -> None: """ @@ -217,6 +223,8 @@ def _apply_goodtimes_filters( Index of the current pointing in l1b_de_datasets. l1b_hk : xr.Dataset L1B housekeeping dataset. + l1a_diagfee : xr.Dataset + L1A DIAG_FEE dataset containing TDC calibration status. cal_product_config_path : Path Path to calibration product configuration CSV file. """ @@ -263,15 +271,19 @@ def _apply_goodtimes_filters( logger.info("Applying filter: mark_drf_times") mark_drf_times(goodtimes_ds, l1b_hk) - # 3. Mark overflow packets + # 3. Mark bad TDC calibration times + logger.info("Applying filter: mark_bad_tdc_cal") + mark_bad_tdc_cal(goodtimes_ds, l1a_diagfee) + + # 4. Mark overflow packets logger.info("Applying filter: mark_overflow_packets") mark_overflow_packets(goodtimes_ds, current_l1b_de, cal_product_config) - # 4. Statistical Filter 0 - drastic background changes + # 5. Statistical Filter 0 - drastic background changes logger.info("Applying filter: mark_statistical_filter_0") mark_statistical_filter_0(goodtimes_ds, l1b_de_datasets, current_index) - # 5. Statistical Filter 1 - isotropic count rate increases + # 6. Statistical Filter 1 - isotropic count rate increases logger.info("Applying filter: mark_statistical_filter_1") mark_statistical_filter_1( goodtimes_ds, @@ -279,7 +291,7 @@ def _apply_goodtimes_filters( current_index, ) - # 6. Statistical Filter 2 - short-lived event pulses + # 7. Statistical Filter 2 - short-lived event pulses logger.info("Applying filter: mark_statistical_filter_2") mark_statistical_filter_2( goodtimes_ds, @@ -1130,6 +1142,92 @@ def mark_overflow_packets( ) +def mark_bad_tdc_cal( + goodtimes_ds: xr.Dataset, + diagfee: xr.Dataset, + check_tdc1: bool = True, + check_tdc2: bool = True, + check_tdc3: bool = True, + cull_code: int = CullCode.LOOSE, +) -> None: + """ + Remove times with failed TDC calibration (DIAG_FEE method). + + Based on C reference: drop_bad_tdc_diagfee in culling.c + + This function scans DIAG_FEE packets chronologically and checks the TDC + calibration status for each packet. If any checked TDC has failed + calibration, all times from that DIAG_FEE packet until the next DIAG_FEE + packet are marked as bad. + + Parameters + ---------- + goodtimes_ds : xr.Dataset + Goodtimes dataset to update with cull flags. + diagfee : xr.Dataset + DIAG_FEE dataset containing TDC calibration status fields: + - shcoarse: Mission Elapsed Time (MET) + - tdc1_cal_ctrl_stat: TDC1 calibration status (bit 1 = success) + - tdc2_cal_ctrl_stat: TDC2 calibration status (bit 1 = success) + - tdc3_cal_ctrl_stat: TDC3 calibration status (bit 1 = success) + check_tdc1 : bool, optional + Whether to check TDC1 calibration status. Default is True. + check_tdc2 : bool, optional + Whether to check TDC2 calibration status. Default is True. + check_tdc3 : bool, optional + Whether to check TDC3 calibration status. Default is True. + cull_code : int, optional + Cull code to use for marking bad times. Default is CullCode.LOOSE. + + Notes + ----- + This function modifies goodtimes_ds in place. + + Quirk: Two DIAG_FEE packets are generated when entering HVSCI mode. + The first packet is skipped if two packets appear within 10 seconds. + """ + logger.info("Running mark_bad_tdc_cal culling") + + # Based on sample code in culling_v2.c, skip this check if we have fewer + # than two diag_fee packets. + if len(diagfee.epoch) < 2: + logger.warning("No DIAG_FEE data to use for selecting good times") + return + + df_met = diagfee["shcoarse"].values + met_values = goodtimes_ds.coords["met"].values + n_times_removed = 0 + + for i in range(len(df_met)): + # Skip duplicate DIAG_FEE packets (within 10 seconds of next) + if i < len(df_met) - 1 and df_met[i] + 10 > df_met[i + 1]: + continue + + # Check TDC calibration status (bit 1: 1=good, 0=bad) + any_tdc_failed = False + + if check_tdc1 and (diagfee["tdc1_cal_ctrl_stat"].values[i] & 2) == 0: + any_tdc_failed = True + if check_tdc2 and (diagfee["tdc2_cal_ctrl_stat"].values[i] & 2) == 0: + any_tdc_failed = True + if check_tdc3 and (diagfee["tdc3_cal_ctrl_stat"].values[i] & 2) == 0: + any_tdc_failed = True + + if any_tdc_failed: + # Remove times from this DIAG_FEE until next + df_time = df_met[i] + next_df_time = df_met[i + 1] if i < len(df_met) - 1 else np.inf + + in_window = (met_values >= df_time) & (met_values < next_df_time) + mets_to_cull = met_values[in_window] + + if len(mets_to_cull) > 0: + goodtimes_ds.goodtimes.mark_bad_times(met=mets_to_cull, cull=cull_code) + n_times_removed += len(mets_to_cull) + + logger.info(f"Dropped {n_times_removed} time(s) due to bad TDC calibration") + + def _get_sweep_indices(esa_step: np.ndarray) -> np.ndarray: """ Assign sweep indices to each MET based on ESA step transitions. diff --git a/imap_processing/tests/hi/test_hi_goodtimes.py b/imap_processing/tests/hi/test_hi_goodtimes.py index 5ef7730bb..c591b8c17 100644 --- a/imap_processing/tests/hi/test_hi_goodtimes.py +++ b/imap_processing/tests/hi/test_hi_goodtimes.py @@ -23,6 +23,7 @@ _identify_cull_pattern, create_goodtimes_dataset, hi_goodtimes, + mark_bad_tdc_cal, mark_drf_times, mark_incomplete_spin_sets, mark_overflow_packets, @@ -1413,6 +1414,255 @@ def test_mark_drf_times_transition_at_end(self): assert n_culled <= 31 # But not all (only last ~30 minutes) +class TestMarkBadTdcCal: + """Test suite for mark_bad_tdc_cal() function.""" + + @pytest.fixture + def goodtimes_for_tdc(self): + """Create a goodtimes dataset with METs spanning a range.""" + # Create METs every 50 seconds for 200 seconds (5 METs) + n_mets = 5 + met_values = np.arange(1000.0, 1000.0 + n_mets * 50, 50) + + gt = xr.Dataset( + { + "cull_flags": xr.DataArray( + np.zeros((n_mets, 90), dtype=np.uint8), dims=["met", "spin_bin"] + ), + "esa_step": xr.DataArray(np.ones(n_mets, dtype=np.uint8), dims=["met"]), + }, + coords={"met": met_values, "spin_bin": np.arange(90)}, + attrs={"sensor": "45sensor", "pointing": 1}, + ) + return gt + + @pytest.fixture + def diagfee_all_good(self): + """Create DIAG_FEE dataset where all TDC calibrations pass.""" + # 4 DIAG_FEE packets, all with bit 1 set (=2, meaning calibration good) + return xr.Dataset( + { + "shcoarse": (["epoch"], np.array([1000, 1050, 1100, 1150])), + "tdc1_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + "tdc2_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + "tdc3_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + } + ) + + @pytest.fixture + def diagfee_tdc1_fails(self): + """Create DIAG_FEE dataset where TDC1 fails at packet index 2.""" + # TDC1 fails at index 2 (bit 1 not set, so value 0) + return xr.Dataset( + { + "shcoarse": (["epoch"], np.array([1000, 1050, 1100, 1150])), + "tdc1_cal_ctrl_stat": ( + ["epoch"], + np.array([2, 2, 0, 2]), + ), # fails at idx 2 + "tdc2_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + "tdc3_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + } + ) + + @pytest.fixture + def diagfee_with_duplicate(self): + """Create DIAG_FEE dataset with duplicate packets within 10 seconds.""" + # First two packets are within 10 seconds (should skip first) + return xr.Dataset( + { + "shcoarse": (["epoch"], np.array([1000, 1005, 1100, 1150])), + "tdc1_cal_ctrl_stat": ( + ["epoch"], + np.array([0, 2, 2, 2]), + ), # First would fail but is skipped + "tdc2_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + "tdc3_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + } + ) + + def test_mark_bad_tdc_cal_all_good(self, goodtimes_for_tdc, diagfee_all_good): + """Test that no times are marked when all TDC calibrations pass.""" + mark_bad_tdc_cal(goodtimes_for_tdc, diagfee_all_good) + + # All times should remain good + assert np.all(goodtimes_for_tdc["cull_flags"].values == CullCode.GOOD) + + def test_mark_bad_tdc_cal_tdc1_fails(self, goodtimes_for_tdc, diagfee_tdc1_fails): + """Test that times are marked when TDC1 fails.""" + mark_bad_tdc_cal(goodtimes_for_tdc, diagfee_tdc1_fails) + + # TDC1 fails at packet 2 (MET 1100), should mark times from 1100 to 1150 + # goodtimes METs are [1000, 1050, 1100, 1150, 1200] + # MET 1100 falls in window [1100, 1150), so MET 1100 should be culled + met_values = goodtimes_for_tdc.coords["met"].values + + # MET 1100 (index 2) should be culled + idx_1100 = np.where(met_values == 1100.0)[0][0] + assert np.all( + goodtimes_for_tdc["cull_flags"].values[idx_1100, :] == CullCode.LOOSE + ) + + # METs before 1100 should still be good + assert np.all( + goodtimes_for_tdc["cull_flags"].values[0, :] == CullCode.GOOD + ) # 1000 + assert np.all( + goodtimes_for_tdc["cull_flags"].values[1, :] == CullCode.GOOD + ) # 1050 + + def test_mark_bad_tdc_cal_skip_duplicate_packets( + self, goodtimes_for_tdc, diagfee_with_duplicate + ): + """Test that duplicate DIAG_FEE packets within 10 seconds are skipped.""" + mark_bad_tdc_cal(goodtimes_for_tdc, diagfee_with_duplicate) + + # First packet (MET 1000) has TDC1 fail but should be skipped + # because it's within 10 seconds of the next packet (MET 1005) + # So all times should remain good + assert np.all(goodtimes_for_tdc["cull_flags"].values == CullCode.GOOD) + + def test_mark_bad_tdc_cal_insufficient_packets(self, goodtimes_for_tdc): + """Test that less than 2 packets logs warning and returns early.""" + # Create DIAG_FEE with only 1 packet + diagfee_single = xr.Dataset( + { + "shcoarse": (["epoch"], np.array([1000])), + "tdc1_cal_ctrl_stat": (["epoch"], np.array([0])), # Fails but ignored + "tdc2_cal_ctrl_stat": (["epoch"], np.array([2])), + "tdc3_cal_ctrl_stat": (["epoch"], np.array([2])), + } + ) + + mark_bad_tdc_cal(goodtimes_for_tdc, diagfee_single) + + # All times should remain good (no culling due to insufficient packets) + assert np.all(goodtimes_for_tdc["cull_flags"].values == CullCode.GOOD) + + def test_mark_bad_tdc_cal_selective_tdc_checking( + self, goodtimes_for_tdc, diagfee_tdc1_fails + ): + """Test that TDC checking can be selectively disabled.""" + # Disable TDC1 checking - should not cull even though TDC1 fails + mark_bad_tdc_cal( + goodtimes_for_tdc, diagfee_tdc1_fails, check_tdc1=False, check_tdc2=True + ) + + # All times should remain good since TDC1 checking is disabled + assert np.all(goodtimes_for_tdc["cull_flags"].values == CullCode.GOOD) + + def test_mark_bad_tdc_cal_tdc2_fails(self, goodtimes_for_tdc): + """Test that times are marked when TDC2 fails.""" + diagfee_tdc2_fails = xr.Dataset( + { + "shcoarse": (["epoch"], np.array([1000, 1050, 1100, 1150])), + "tdc1_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + "tdc2_cal_ctrl_stat": ( + ["epoch"], + np.array([2, 0, 2, 2]), + ), # fails at idx 1 + "tdc3_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + } + ) + + mark_bad_tdc_cal(goodtimes_for_tdc, diagfee_tdc2_fails) + + # TDC2 fails at packet 1 (MET 1050), should mark times from 1050 to 1100 + met_values = goodtimes_for_tdc.coords["met"].values + + # MET 1050 (index 1) should be culled + idx_1050 = np.where(met_values == 1050.0)[0][0] + assert np.all( + goodtimes_for_tdc["cull_flags"].values[idx_1050, :] == CullCode.LOOSE + ) + + def test_mark_bad_tdc_cal_tdc3_fails(self, goodtimes_for_tdc): + """Test that times are marked when TDC3 fails.""" + diagfee_tdc3_fails = xr.Dataset( + { + "shcoarse": (["epoch"], np.array([1000, 1050, 1100, 1150])), + "tdc1_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + "tdc2_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + "tdc3_cal_ctrl_stat": ( + ["epoch"], + np.array([0, 2, 2, 2]), + ), # fails at idx 0 + } + ) + + mark_bad_tdc_cal(goodtimes_for_tdc, diagfee_tdc3_fails) + + # TDC3 fails at packet 0 (MET 1000), should mark times from 1000 to 1050 + # MET 1000 (index 0) should be culled + assert np.all( + goodtimes_for_tdc["cull_flags"].values[0, :] == CullCode.LOOSE + ) # 1000 + + # MET 1050 should be good (next DIAG_FEE packet starts good window) + assert np.all( + goodtimes_for_tdc["cull_flags"].values[1, :] == CullCode.GOOD + ) # 1050 + + def test_mark_bad_tdc_cal_custom_cull_code( + self, goodtimes_for_tdc, diagfee_tdc1_fails + ): + """Test that custom cull code is used.""" + custom_cull_code = 5 + mark_bad_tdc_cal( + goodtimes_for_tdc, diagfee_tdc1_fails, cull_code=custom_cull_code + ) + + # Check that culled times use custom code + assert np.any(goodtimes_for_tdc["cull_flags"].values == custom_cull_code) + + def test_mark_bad_tdc_cal_last_packet_fails(self, goodtimes_for_tdc): + """Test behavior when the last DIAG_FEE packet has TDC failure.""" + diagfee_last_fails = xr.Dataset( + { + "shcoarse": (["epoch"], np.array([1000, 1050, 1100, 1150])), + "tdc1_cal_ctrl_stat": ( + ["epoch"], + np.array([2, 2, 2, 0]), + ), # fails at last + "tdc2_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + "tdc3_cal_ctrl_stat": (["epoch"], np.array([2, 2, 2, 2])), + } + ) + + mark_bad_tdc_cal(goodtimes_for_tdc, diagfee_last_fails) + + # TDC1 fails at last packet (MET 1150), should mark all times >= 1150 + met_values = goodtimes_for_tdc.coords["met"].values + + # METs >= 1150 should be culled + for i, met in enumerate(met_values): + if met >= 1150: + assert np.all( + goodtimes_for_tdc["cull_flags"].values[i, :] == CullCode.LOOSE + ) + else: + assert np.all( + goodtimes_for_tdc["cull_flags"].values[i, :] == CullCode.GOOD + ) + + def test_mark_bad_tdc_cal_empty_diagfee(self, goodtimes_for_tdc): + """Test that function handles empty DIAG_FEE data gracefully.""" + diagfee_empty = xr.Dataset( + { + "shcoarse": (["epoch"], np.array([], dtype=np.float64)), + "tdc1_cal_ctrl_stat": (["epoch"], np.array([], dtype=np.uint8)), + "tdc2_cal_ctrl_stat": (["epoch"], np.array([], dtype=np.uint8)), + "tdc3_cal_ctrl_stat": (["epoch"], np.array([], dtype=np.uint8)), + } + ) + + # Should log warning and return without error + mark_bad_tdc_cal(goodtimes_for_tdc, diagfee_empty) + + # All times should remain good + assert np.all(goodtimes_for_tdc["cull_flags"].values == CullCode.GOOD) + + class TestMarkOverflowPackets: """Test suite for mark_overflow_packets function.""" @@ -3407,13 +3657,14 @@ def test_loads_cal_config(self, tmp_path): [mock_l1b_de], current_index=0, l1b_hk=mock_hk, + l1a_diagfee=MagicMock(), cal_product_config_path=cal_path, ) mock_cal_load.assert_called_once_with(cal_path) def test_calls_all_filters(self, tmp_path): - """Test that all 6 filters are called.""" + """Test that all 7 filters are called.""" mock_goodtimes = MagicMock() mock_goodtimes.goodtimes.get_cull_statistics.return_value = { "good_bins": 100, @@ -3432,22 +3683,24 @@ def test_calls_all_filters(self, tmp_path): "imap_processing.hi.hi_goodtimes.mark_incomplete_spin_sets" ) as mock_f1, patch("imap_processing.hi.hi_goodtimes.mark_drf_times") as mock_f2, - patch("imap_processing.hi.hi_goodtimes.mark_overflow_packets") as mock_f3, + patch("imap_processing.hi.hi_goodtimes.mark_bad_tdc_cal") as mock_f3, + patch("imap_processing.hi.hi_goodtimes.mark_overflow_packets") as mock_f4, patch( "imap_processing.hi.hi_goodtimes.mark_statistical_filter_0" - ) as mock_f4, + ) as mock_f5, patch( "imap_processing.hi.hi_goodtimes.mark_statistical_filter_1" - ) as mock_f5, + ) as mock_f6, patch( "imap_processing.hi.hi_goodtimes.mark_statistical_filter_2" - ) as mock_f6, + ) as mock_f7, ): _apply_goodtimes_filters( mock_goodtimes, [mock_l1b_de], current_index=0, l1b_hk=mock_hk, + l1a_diagfee=MagicMock(), cal_product_config_path=tmp_path / "cal.csv", ) @@ -3457,6 +3710,7 @@ def test_calls_all_filters(self, tmp_path): mock_f4.assert_called_once() mock_f5.assert_called_once() mock_f6.assert_called_once() + mock_f7.assert_called_once() def test_raises_statistical_filter_0_errors(self, tmp_path): """Test that ValueError from statistical filter 0 is raised.""" @@ -3476,6 +3730,7 @@ def test_raises_statistical_filter_0_errors(self, tmp_path): ), patch("imap_processing.hi.hi_goodtimes.mark_incomplete_spin_sets"), patch("imap_processing.hi.hi_goodtimes.mark_drf_times"), + patch("imap_processing.hi.hi_goodtimes.mark_bad_tdc_cal"), patch("imap_processing.hi.hi_goodtimes.mark_overflow_packets"), patch( "imap_processing.hi.hi_goodtimes.mark_statistical_filter_0", @@ -3488,6 +3743,7 @@ def test_raises_statistical_filter_0_errors(self, tmp_path): [mock_l1b_de], current_index=0, l1b_hk=mock_hk, + l1a_diagfee=MagicMock(), cal_product_config_path=tmp_path / "cal.csv", ) @@ -3509,6 +3765,7 @@ def test_raises_statistical_filter_1_errors(self, tmp_path): ), patch("imap_processing.hi.hi_goodtimes.mark_incomplete_spin_sets"), patch("imap_processing.hi.hi_goodtimes.mark_drf_times"), + patch("imap_processing.hi.hi_goodtimes.mark_bad_tdc_cal"), patch("imap_processing.hi.hi_goodtimes.mark_overflow_packets"), patch("imap_processing.hi.hi_goodtimes.mark_statistical_filter_0"), patch( @@ -3522,6 +3779,7 @@ def test_raises_statistical_filter_1_errors(self, tmp_path): [mock_l1b_de], current_index=0, l1b_hk=mock_hk, + l1a_diagfee=MagicMock(), cal_product_config_path=tmp_path / "cal.csv", ) @@ -3547,9 +3805,10 @@ def test_raises_value_error_when_repoint_not_complete(self, tmp_path): ValueError, match="Goodtimes cannot yet be processed for repoint00001" ): _ = hi_goodtimes( - l1b_de_datasets=[mock_de], current_repointing="repoint00001", + l1b_de_datasets=[mock_de], l1b_hk=mock_hk, + l1a_diagfee=MagicMock(), cal_product_config_path=tmp_path / "cal.csv", ) @@ -3587,9 +3846,10 @@ def test_calls_find_current_index_when_repoint_complete(self, tmp_path): patch("imap_processing.hi.hi_goodtimes._apply_goodtimes_filters"), ): hi_goodtimes( - l1b_de_datasets=mock_datasets, current_repointing="repoint00004", + l1b_de_datasets=mock_datasets, l1b_hk=mock_hk, + l1a_diagfee=MagicMock(), cal_product_config_path=tmp_path / "cal.csv", ) @@ -3629,9 +3889,10 @@ def test_marks_all_bad_when_incomplete_de_set(self, tmp_path): ), ): hi_goodtimes( - l1b_de_datasets=mock_datasets, current_repointing="repoint00001", + l1b_de_datasets=mock_datasets, l1b_hk=mock_hk, + l1a_diagfee=MagicMock(), cal_product_config_path=tmp_path / "cal.csv", ) @@ -3673,9 +3934,10 @@ def test_calls_apply_filters_when_full_de_set(self, tmp_path): ) as mock_apply, ): hi_goodtimes( - l1b_de_datasets=mock_datasets, current_repointing="repoint00004", + l1b_de_datasets=mock_datasets, l1b_hk=mock_hk, + l1a_diagfee=MagicMock(), cal_product_config_path=tmp_path / "cal.csv", ) @@ -3715,9 +3977,10 @@ def test_returns_datasets(self, tmp_path): patch("imap_processing.hi.hi_goodtimes._apply_goodtimes_filters"), ): result = hi_goodtimes( - l1b_de_datasets=mock_datasets, current_repointing="repoint00004", + l1b_de_datasets=mock_datasets, l1b_hk=mock_hk, + l1a_diagfee=MagicMock(), cal_product_config_path=tmp_path / "cal.csv", ) From 02eec54fd688eb84358585707efccba571bb1cee Mon Sep 17 00:00:00 2001 From: Tim Plummer Date: Wed, 11 Mar 2026 11:33:42 -0600 Subject: [PATCH 3/3] Copilot feedback changes --- imap_processing/hi/hi_goodtimes.py | 7 +++++-- imap_processing/tests/hi/test_hi_goodtimes.py | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/imap_processing/hi/hi_goodtimes.py b/imap_processing/hi/hi_goodtimes.py index 0d2347abd..3eb84d956 100644 --- a/imap_processing/hi/hi_goodtimes.py +++ b/imap_processing/hi/hi_goodtimes.py @@ -211,7 +211,7 @@ def _apply_goodtimes_filters( """ Apply all goodtimes culling filters to the dataset. - Modifies goodtimes_ds in place by applying filters 1-6. + Modifies goodtimes_ds in place by applying filters 1-7. Parameters ---------- @@ -1191,7 +1191,10 @@ def mark_bad_tdc_cal( # Based on sample code in culling_v2.c, skip this check if we have fewer # than two diag_fee packets. if len(diagfee.epoch) < 2: - logger.warning("No DIAG_FEE data to use for selecting good times") + logger.warning( + f"Insufficient DIAG_FEE packets to select good times " + f"(found {len(diagfee.epoch)}, need at least 2)" + ) return df_met = diagfee["shcoarse"].values diff --git a/imap_processing/tests/hi/test_hi_goodtimes.py b/imap_processing/tests/hi/test_hi_goodtimes.py index c591b8c17..b99454342 100644 --- a/imap_processing/tests/hi/test_hi_goodtimes.py +++ b/imap_processing/tests/hi/test_hi_goodtimes.py @@ -3646,6 +3646,7 @@ def test_loads_cal_config(self, tmp_path): patch("imap_processing.hi.hi_goodtimes.mark_incomplete_spin_sets"), patch("imap_processing.hi.hi_goodtimes.mark_drf_times"), patch("imap_processing.hi.hi_goodtimes.mark_overflow_packets"), + patch("imap_processing.hi.hi_goodtimes.mark_bad_tdc_cal"), patch("imap_processing.hi.hi_goodtimes.mark_statistical_filter_0"), patch("imap_processing.hi.hi_goodtimes.mark_statistical_filter_1"), patch("imap_processing.hi.hi_goodtimes.mark_statistical_filter_2"),