From c2eee1e2d7d2b02fb8a83083fdc90a3ec8087092 Mon Sep 17 00:00:00 2001 From: arjunsridhar12345 Date: Wed, 12 Aug 2026 15:27:22 -0700 Subject: [PATCH 1/3] fix: set min reward to 0 by default --- src/dynamic_foraging_processing/processing/_trial_table.py | 6 +++--- .../processing/models/trial_config.py | 7 +++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/dynamic_foraging_processing/processing/_trial_table.py b/src/dynamic_foraging_processing/processing/_trial_table.py index fd6a892..724828b 100644 --- a/src/dynamic_foraging_processing/processing/_trial_table.py +++ b/src/dynamic_foraging_processing/processing/_trial_table.py @@ -658,9 +658,9 @@ def _session_columns(self, task_logic: AindDynamicForagingTaskLogic) -> t.Dict[s delay_max=delay_max, base_reward_probability_sum=base_reward_sum, ) - # ``min_block_reward`` is warmup-generator-only; main coupled generators omit it. - if hasattr(generator, "min_block_reward"): - columns["min_reward_each_block"] = generator.min_block_reward + # ``min_block_reward`` is warmup-generator-only; a generator that omits it + # enforces no per-block minimum, which is a floor of 0 rather than unknown. + columns["min_reward_each_block"] = getattr(generator, "min_block_reward", 0) return columns def _manipulator_mm_per_step(self, rig: AindDynamicForagingRig) -> t.Dict[str, float]: diff --git a/src/dynamic_foraging_processing/processing/models/trial_config.py b/src/dynamic_foraging_processing/processing/models/trial_config.py index 46af23f..22e0e96 100644 --- a/src/dynamic_foraging_processing/processing/models/trial_config.py +++ b/src/dynamic_foraging_processing/processing/models/trial_config.py @@ -139,8 +139,11 @@ class TrialConfig(BaseModel): block_max: Optional[float] = Field( default=None, description="The maximum length allowed for each block" ) - min_reward_each_block: Optional[float] = Field( - default=None, description="The minimum reward allowed for each block" + min_reward_each_block: float = Field( + default=0, + description=( + "The minimum reward allowed for each block; 0 when the generator enforces no per-block minimum (only the warmup generator does)" + ), ) # --- delay_duration --- From 708e59f57126d5a7853e4d65045172ba5321a6af Mon Sep 17 00:00:00 2001 From: arjunsridhar12345 Date: Wed, 12 Aug 2026 15:27:31 -0700 Subject: [PATCH 2/3] test: update tests --- tests/test_processing/test_trial_table.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_processing/test_trial_table.py b/tests/test_processing/test_trial_table.py index 90de39a..b213ba1 100644 --- a/tests/test_processing/test_trial_table.py +++ b/tests/test_processing/test_trial_table.py @@ -365,7 +365,8 @@ def test_build_full_dataset(): # Scalar has neither a scale nor truncation parameters -> null bounds. assert pd.isna(first["delay_min"]) assert pd.isna(first["delay_max"]) - assert pd.isna(first["min_reward_each_block"]) # removed from generator schema + # No per-block reward minimum on this generator -> a floor of 0, not null. + assert first["min_reward_each_block"] == 0 assert first["base_reward_probability_sum"] == pytest.approx(0.8) # Lickspout positions from AccumulatedSteps (microsteps * 0.00125 mm), @@ -563,7 +564,8 @@ def test_session_columns_uncoupled_has_null_reward_sum(): assert "ITI_beta" in columns # Coupled-only fields are absent / null for an uncoupled generator. assert columns["base_reward_probability_sum"] is None - assert "min_reward_each_block" not in columns + # No ``min_block_reward`` on this generator -> no per-block minimum (0). + assert columns["min_reward_each_block"] == 0 # --------------------------------------------------------------------------- # From 2d4167c06b68464ddaabbe2bfed0fffee791843b Mon Sep 17 00:00:00 2001 From: arjunsridhar12345 Date: Wed, 12 Aug 2026 15:27:41 -0700 Subject: [PATCH 3/3] docs: update changelog --- docs/trials_table_mapping.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/trials_table_mapping.md b/docs/trials_table_mapping.md index acce921..c69f3db 100644 --- a/docs/trials_table_mapping.md +++ b/docs/trials_table_mapping.md @@ -70,14 +70,14 @@ Columns are grouped by the raw source they map from. | `reward_probabilityL` / `reward_probabilityR` | The **block** probability from `Trial -> metadata -> p_reward_left` / `p_reward_right`. The top-level `trial.p_reward_left` / `p_reward_right` is the per-trial probability, not the block probability, so it is not used here. `None` when the trial or its metadata is missing. | | `reward_size_left` | `Trial -> reward_size.left` — the reward volume (uL) at the left port. Defaults to `2.0` when not set on the trial. `None` when the trial is missing. | | `reward_size_right` | `Trial -> reward_size.right` — the reward volume (uL) at the right port. Defaults to `2.0` when not set on the trial. `None` when the trial is missing. | -| `rewarded_historyL` / `rewarded_historyR` | Filter `is_rewarded == True`, then on `is_right_choice`. | +| `rewarded_historyL` / `rewarded_historyR` | **Earned** reward only: filter `is_rewarded == True`, then on `is_right_choice`. `False` on both sides when `is_auto_reward_right` is set (either side) — that trial's water is autowater and is reported by `auto_waterL` / `auto_waterR`. | ### From `TrialGeneratorSpec.json` (`SoftwareEvents` stream) | Trials column | Mapping | | --- | --- | | `base_reward_probability_sum` | If `type == "CoupledTrialGenerator"`, look at `reward_probability_parameters`. | -| `min_reward_each_block` | Present when `type == "CoupledWarmupTrialGenerator"` (has `min_block_reward`); otherwise `None`. | +| `min_reward_each_block` | `min_block_reward` when `type == "CoupledWarmupTrialGenerator"`; otherwise `0`, since a generator without that field enforces no per-block reward minimum. | ### Trial period timing (the four period `SoftwareEvents` streams) @@ -184,8 +184,10 @@ These were mapped during exploration but are no longer in scope: | 2026-06-17 | `auto_waterL` / `auto_waterR` now encode no auto-response (`is_auto_reward_right` is `None`) and missing trials as `0` instead of `NULL`. The columns are non-nullable (`int`, default `0`). | | 2026-06-20 | Added `reward_size_left` / `reward_size_right` (reward volume in uL) from `task_parameters.reward_size`, and `side_bias` from the per-trial `TrialMetrics` event (`bias` field). | | 2026-06-20 | `reward_probabilityL` / `reward_probabilityR` now read the block probability from `trial.metadata.p_reward_left` / `p_reward_right` instead of the top-level per-trial `trial.p_reward_left` / `p_reward_right`. | -| 2026-07-27 | Added `anti_bias_left_water` / `anti_bias_right_water` (boolean anti-bias water interventions per side) and `anti_bias_lickspout_movement` (mm the anti-bias algorithm shifted the lickspouts) from `TrialOutcome`'s `trial.metadata.extra` (`is_bias_water_intervention` / `is_bias_stage_intervention`), `is_auto_reward_right`, and `lickspout_offset_delta`. These are also overlaid on the QC `side_bias.png` figure. | | 2026-07-22 | `lickspout_position_x` / `y1` / `y2` / `z` now derive from the `HarpManipulator` `AccumulatedSteps` stream (microsteps → mm via the `InputSchemas.Rig` manipulator calibration, `full_step_to_mm / microstep_resolution`), sampled per trial via the closest sample in the `[start_time, stop_time)` window and re-referenced to the session-start position (displacement relative to session start, mm), replacing the static `InitialManipulatorPosition` software event. `Motor{i}` maps to `Axis(i + 1)` (X, Y1, Y2, Z). The rig and `AccumulatedSteps` streams are required when there are trials (`build` raises if either is missing). Column descriptions corrected from `um` to `mm`. | | 2026-07-24 | `reward_size_left` / `reward_size_right` moved from session-level `task_parameters.reward_size` to per-trial `Trial.reward_size` (fields `.left` / `.right`). The columns are now nullable — `None` when the trial is missing. A missing `TaskLogic` stream no longer raises; session distribution columns are simply null. `min_reward_each_block` moved from `CoupledTrialGenerator` to `CoupledWarmupTrialGenerator`. | +| 2026-07-27 | Added `anti_bias_left_water` / `anti_bias_right_water` (boolean anti-bias water interventions per side) and `anti_bias_lickspout_movement` (mm the anti-bias algorithm shifted the lickspouts) from `TrialOutcome`'s `trial.metadata.extra` (`is_bias_water_intervention` / `is_bias_stage_intervention`), `is_auto_reward_right`, and `lickspout_offset_delta`. These are also overlaid on the QC `side_bias.png` figure. | | 2026-08-06 | **Breaking:** the trial `start_time` / `stop_time` columns are removed and replaced by one start/stop pair per task period: `quiescent_start_time` / `quiescent_stop_time`, `response_start_time` / `response_stop_time`, `reward_consumption_start_time` / `reward_consumption_stop_time`, and `ITI_start_time` / `ITI_stop_time`, read from the `ResponsePeriod` and `RewardConsumptionPeriod` streams in addition to `QuiescentPeriod` and `ItiPeriod`. Each period event marks its period's start, so each stop is the next period's start; `ITI_stop_time` is the next trial's `QuiescentPeriod` timestamp (`NaN` on the last trial). The two new streams are also checked for positional alignment with `TrialOutcome`. NWB's required native `start_time` / `stop_time` are now derived when writing (`quiescent_start_time` → `ITI_stop_time`, falling back to `ITI_start_time`), so the NWB trials table changes in two ways: the old `start_time` / `stop_time` columns are gone, and the native trial extent now ends at the *end* of the ITI rather than at its start. | | 2026-08-06 | Confirmed and documented that the legacy `delay_*` columns describe the acquisition software's **quiescence period**: `delay_start_time` is the `QuiescentPeriod` timestamp (always equal to the new `quiescent_start_time`) and `delay_duration` / `delay_beta` / `delay_min` / `delay_max` summarize `quiescence_period_duration`. `delay_duration` is the *configured* duration — each lick restarts the quiescent period, so the realized `quiescent_stop_time - quiescent_start_time` can be longer. Column descriptions updated accordingly. | +| 2026-08-12 | `rewarded_historyL` / `rewarded_historyR` now record **earned** reward only: an auto-reward trial (`is_auto_reward_right` set to either side) is `False` on *both* sides, since `TrialOutcome.is_rewarded` is `True` for autowater too and that water is already reported by `auto_waterL` / `auto_waterR`. This matches the `earned` / `automatic` split used for the NWB reward-delivery annotations. | +| 2026-08-12 | `min_reward_each_block` is now `0` rather than `NULL` when the trial generator exposes no `min_block_reward` — no per-block minimum is a floor of zero, not an unknown. The column is non-nullable (`float`, default `0`). |