Add QA-7 quantum plasticity evaluation and close quantum architecture campaign - #80
Merged
Conversation
📝 WalkthroughWalkthroughAdds a plasticity evaluation capability: new per-architecture YAML study configs, a plasticity package (types, metrics, snapshot), Pydantic config loaders, CLI scripts to run/compare seeded multi-phase plasticity tests with checkpointing and exports, experiment-metadata extensions, tests, a dependency add (scipy), and documentation/specs recording QA-7 results and a strategic pivot. Changes
Sequence Diagram(s)sequenceDiagram
participant Script as run_plasticity_test.py
participant Brain
participant Agent as QuantumNematodeAgent
participant Env as Environment
participant Checkpoint
participant Metrics
rect rgba(100,200,150,0.5)
Note over Script: Per-seed initialization
Script->>Brain: build_brain_model(seed)
end
rect rgba(100,150,200,0.5)
Note over Script: Phase loop (A → B → C → A')
loop For each phase
Script->>Agent: build_agent_for_phase(phase_config)
Script->>Checkpoint: snapshot_brain_state()
loop Training episodes
Agent->>Env: step(action)
Env->>Agent: observation, reward
Agent->>Brain: update_policy()
end
Script->>Checkpoint: save_checkpoint(phase_name)
rect rgba(200,150,100,0.5)
Note over Script: Evaluation block (snapshot/restore)
Script->>Checkpoint: snapshot_brain_state()
loop Eval episodes
Agent->>Env: step(action)
Env->>Agent: observation, reward
end
Script->>Checkpoint: restore_brain_state()
Script->>Metrics: collect_eval_result(transition)
end
end
end
rect rgba(150,100,200,0.5)
Note over Script: Per-seed metrics
Script->>Metrics: compute_convergence_episode()
Script->>Metrics: compute_seed_metrics()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
packages/quantum-nematode/quantumnematode/plasticity/snapshot.py (1)
60-63: Fail fast on partial restore mismatches.Lines 60-63 silently skip snapshot entries that no longer exist on the brain object. That can hide partial restores and invalidate “no-trace” eval guarantees. Prefer raising on missing keys.
Suggested strict-restore patch
def restore_brain_state( brain: Brain, snapshot: dict[str, dict[str, Any]], ) -> None: """Restore brain state from a snapshot and clear any PPO buffer.""" modules = _get_torch_modules(brain) + missing = [name for name in snapshot if name not in modules] + if missing: + raise ValueError(f"Snapshot contains unknown module/optimizer keys: {missing}") + for name, state_dict in snapshot.items(): - if name in modules: - modules[name].load_state_dict(deepcopy(state_dict)) + modules[name].load_state_dict(deepcopy(state_dict))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/quantum-nematode/quantumnematode/plasticity/snapshot.py` around lines 60 - 63, The loop that restores state_dicts currently skips snapshot entries not present in modules (for name, state_dict in snapshot.items()) which hides partial restores; change it to fail-fast by checking each snapshot key against modules and raising a clear exception (e.g., ValueError or KeyError) when name is missing instead of silently continuing, while still calling modules[name].load_state_dict(deepcopy(state_dict)) for valid keys; ensure the error message references the missing snapshot key and that this check happens before any load_state_dict calls.packages/quantum-nematode/tests/quantumnematode_tests/utils/test_plasticity.py (1)
230-255: Exercise more than the actor state in this test.
scripts/run_plasticity_test.pyrelies onrestore_brain_state()to leave training untouched after each eval block, but this test only comparessnapshot["actor"]. A regression in critic, optimizer, or buffer restoration would still pass here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/quantum-nematode/tests/quantumnematode_tests/utils/test_plasticity.py` around lines 230 - 255, The test currently only verifies actor weights; expand it to snapshot and verify critic parameters, optimizer state, and replay buffer (or training buffer) so restore_brain_state() fully restores training state. Specifically: capture critic params via snapshot_brain_state(brain)["critic"], verify they change after modification using brain.critic.parameters(), include optimizer state by calling brain.optimizer.state_dict() in the snapshot and compare before/after, and include the replay buffer state (e.g., brain.replay_buffer or brain.buffer contents/state) in the snapshot and compare; after restore_brain_state(brain, snapshot) assert that snapshot["critic"], snapshot["optimizer"], and snapshot["buffer"] (or the actual buffer attribute) are equal to the restored ones, and also ensure any training/eval mode flags on brain are restored if snapshot_brain_state records them.packages/quantum-nematode/quantumnematode/plasticity/dtypes.py (1)
8-38: UseBaseModelfor these shared result carriers.These types now cross module boundaries through metrics, the runner, CSV export, and tests. Keeping them as dataclasses makes them inconsistent with the rest of the config/metadata layer and drops the validation/serialization helpers the repo standardizes on.
♻️ Suggested direction
-from dataclasses import dataclass, field +from pydantic import BaseModel, Field -@dataclass -class EvalResult: +class EvalResult(BaseModel): """Metrics from a single evaluation block.""" - objective_name: str transition_point: str mean_success_rate: float mean_reward: float mean_steps: float -@dataclass -class PhaseTrainingResult: +class PhaseTrainingResult(BaseModel): """Per-episode training metrics for a single phase.""" - phase_name: str - episode_successes: list[bool] = field(default_factory=list) - episode_rewards: list[float] = field(default_factory=list) - episode_steps: list[int] = field(default_factory=list) + episode_successes: list[bool] = Field(default_factory=list) + episode_rewards: list[float] = Field(default_factory=list) + episode_steps: list[int] = Field(default_factory=list) -@dataclass -class SeedResult: +class SeedResult(BaseModel): """All results for a single seed run.""" - seed: int - training_results: list[PhaseTrainingResult] = field(default_factory=list) - eval_results: list[EvalResult] = field(default_factory=list) + training_results: list[PhaseTrainingResult] = Field(default_factory=list) + eval_results: list[EvalResult] = Field(default_factory=list) backward_forgetting: float | None = None forward_transfer: float | None = None plasticity_retention: float | None = NoneAs per coding guidelines "Use Pydantic BaseModel for all data structures".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/quantum-nematode/quantumnematode/plasticity/dtypes.py` around lines 8 - 38, Replace the dataclasses EvalResult, PhaseTrainingResult, and SeedResult with Pydantic models by having each class inherit from pydantic.BaseModel (import BaseModel, Field from pydantic) and replace dataclass fields with typed attributes using typing.List and typing.Optional where appropriate (e.g., episode_successes: List[bool] = Field(default_factory=list), episode_rewards: List[float] = Field(default_factory=list), episode_steps: List[int] = Field(default_factory=list), backward_forgetting: Optional[float] = None, etc.); keep the same field names and types (mean_success_rate, mean_reward, mean_steps as float; phase_name, objective_name, transition_point, seed types unchanged) and add any Model Config you need (e.g., orm_mode = True) so these models gain Pydantic validation/serialization behavior used across metrics, runner, CSV export, and tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/experiments/logbooks/008-quantum-brain-evaluation.md`:
- Line 3: The document contains two conflicting campaign session counts ("300+
sessions" in the Status header and "290+ sessions" later); pick the correct
unified count/range and replace the other occurrence so both the Status line
(`**Status**: ... 300+ sessions`) and the later mention that currently reads
"290+ sessions" match exactly; search for both literal strings "300+ sessions"
and "290+ sessions" in the file and update them to the chosen value, leaving the
rest of the text unchanged.
- Around line 1890-1903: The docs claim "4 seeds × 3 architectures = 12
sessions" but the current plasticity configs (mlpppo_plasticity.yml,
crh_plasticity.yml, hybridquantum_plasticity.yml, qrh_plasticity.yml) define 8
seeds each; either update the documentation to reflect "8 seeds × 3
architectures = 24 sessions" and replace the per-seed Backward Forgetting lists
with the full 8-entry vectors and recomputed means/stds, or change the seed
settings in those config files to 4 seeds to match the docs; specifically edit
the header line "Classical Baseline Results (...)" and the per-seed lists in the
markdown (and/or the seed count fields in the named YAML configs) so both docs
and configs agree.
In `@docs/research/quantum-architectures.md`:
- Line 5: Update the document header metadata to reflect the newest changes:
change the "Last Updated" date from 2026-03-20 to 2026-03-21 and update the
summary phrase to include the QA-7 update (e.g., "QA-7 complete, QA-6 deferred,
strategic pivot to environment enrichment") so the header matches the body
edits; locate and modify the header line that currently reads "**Last Updated**:
2026-03-20 (QA-5 complete, QA-6 deferred, strategic pivot to environment
enrichment)" accordingly.
In `@openspec/specs/plasticity-evaluation/spec.md`:
- Around line 108-113: Update the spec to match shipped behavior by making
PlasticityPhaseConfig.reward a required field and constraining plasticity.phases
to the exact four phase entries expected by scripts/run_plasticity_test.py:
foraging, pursuit_predators, thermotaxis_pursuit, and foraging_return (in that
order / all present), rather than "at least 3 phases"; keep
plasticity.convergence_threshold defaulting to 0.6 as-is.
In `@packages/quantum-nematode/quantumnematode/plasticity/metrics.py`:
- Around line 36-37: Change the convergence comparator from non‑strict to
strict: inside the convergence check that currently uses "np.mean(trailing) >=
threshold" (in metrics.py, the block returning "i + 1" for the 1‑indexed
episode), update the condition to use ">" so convergence is only reported when
the trailing-window mean strictly exceeds the threshold.
In `@packages/quantum-nematode/quantumnematode/utils/config_loader.py`:
- Around line 556-563: Ensure PlasticityProtocolConfig validates that
convergence_threshold is a probability (0 < convergence_threshold <= 1) and that
the phases list contains the required phase sequence expected by
plasticity/metrics.py: "foraging" then "pursuit_predators" then
"foraging_return" in that order; implement this by adding a Pydantic field
validator on convergence_threshold to enforce the numeric bounds and a root or
field validator on phases (in PlasticityProtocolConfig) that checks the list of
PlasticityPhaseConfig names for presence and ordering of those three phase names
and raises a clear ValidationError if missing or out-of-order so
compute_convergence_episode and the metrics code cannot silently fail.
In
`@packages/quantum-nematode/tests/quantumnematode_tests/utils/test_plasticity.py`:
- Around line 94-108: Update the test_missing_brain_rejected to assert the
specific Pydantic validation error instead of a broad Exception: replace
pytest.raises(Exception) with pytest.raises(ValidationError) (import
ValidationError from pydantic) so the PlasticityConfig/PlasticityProtocolConfig
constructor call is expected to raise pydantic.ValidationError; ensure the
import is added and adjust the context manager accordingly around the
PlasticityConfig instantiation.
In `@scripts/compare_plasticity_results.py`:
- Around line 103-110: The code currently overwrites earlier runs because
all_metrics is a dict mapping metrics.name to a single ArchMetrics; change
all_metrics to map each metrics.name to a list of ArchMetrics (e.g., dict[str,
list[ArchMetrics]]), and in the loop where you call load_aggregate_csv append
metrics to all_metrics[metrics.name] (creating the list if missing) and update
the console.print to indicate multiple files were loaded for the same
architecture; then adjust downstream logic (places that iterate over
all_metrics, any summary generation or t-test code) to either merge those lists
into a single combined ArchMetrics or perform statistics across the list of runs
so no earlier runs are silently dropped (refer to the variables all_metrics,
metrics.name, load_aggregate_csv and places that consume all_metrics).
- Around line 171-180: Short-circuit degenerate BF samples before computing FR
and significance: if c.bf_mean == 0 set FR handling so it doesn't become inf and
if either q.bf_values or c.bf_values is constant (zero variance) avoid calling
ttest_ind directly (it can return nan) and instead set a deterministic verdict
based on the constant means (e.g., confirmed if q.mean much lower per existing
FR rule, or not significant if equal); for the non-degenerate path call
scipy.stats.ttest_ind(q.bf_values, c.bf_values, equal_var=False) (Welch’s
t-test) and assign p_value from its result, then use fr, p_value, and existing
thresholds to set verdict. Ensure you reference fr, p_value, ttest_ind,
q.bf_values and c.bf_values when making these changes and guard against nan
p_values before comparisons.
In `@scripts/run_plasticity_test.py`:
- Around line 569-575: The runner currently writes CSVs but never creates or
saves the new metadata objects; after write_aggregate_csv and before returning,
construct an ExperimentMetadata (and embedded PlasticityMetadata) using the run
parameters and all_seed_results (use the ExperimentMetadata and
PlasticityMetadata classes from quantumnematode.experiment.metadata) and persist
it to the same export_base (e.g., export_base / "metadata.json" or the project’s
canonical metadata path). Update the block around
write_aggregate_csv/print_summary in run_plasticity_test.py to build the
metadata object (populating fields from all_seed_results, brain_name and any
local run config) and call the metadata serializer/save method used elsewhere in
the codebase so plasticity runs are recorded alongside aggregate_metrics.csv.
- Around line 172-205: The code currently snapshots the brain with
snapshot_brain(brain) but only calls restore_brain_state(brain, snapshot) after
the episode loop, so exceptions from build_agent_for_phase or agent.run_episode
can skip restoration; fix by enclosing the per-episode loop (and any setup that
mutates the brain like build_agent_for_phase and agent.run_episode) in a
try/finally where restore_brain_state(brain, snapshot) is called in the finally
block; ensure snapshot is taken before the try and handle the case where
snapshot might be None so restore_brain_state is only called with a valid
snapshot.
- Around line 462-463: The code constructs BrainType from the raw YAML value
(BrainType(config.brain.name)) even though configure_brain(sim_config)
canonicalizes aliases using BRAIN_NAME_ALIASES; update the construction to use
the canonicalized name returned by configure_brain (e.g., brain_config.name) or
explicitly map config.brain.name through BRAIN_NAME_ALIASES before calling
BrainType so aliases are resolved and ValueError is avoided; refer to
configure_brain, BRAIN_NAME_ALIASES, brain_config, BrainType, and
config.brain.name when making the change.
---
Nitpick comments:
In `@packages/quantum-nematode/quantumnematode/plasticity/dtypes.py`:
- Around line 8-38: Replace the dataclasses EvalResult, PhaseTrainingResult, and
SeedResult with Pydantic models by having each class inherit from
pydantic.BaseModel (import BaseModel, Field from pydantic) and replace dataclass
fields with typed attributes using typing.List and typing.Optional where
appropriate (e.g., episode_successes: List[bool] = Field(default_factory=list),
episode_rewards: List[float] = Field(default_factory=list), episode_steps:
List[int] = Field(default_factory=list), backward_forgetting: Optional[float] =
None, etc.); keep the same field names and types (mean_success_rate,
mean_reward, mean_steps as float; phase_name, objective_name, transition_point,
seed types unchanged) and add any Model Config you need (e.g., orm_mode = True)
so these models gain Pydantic validation/serialization behavior used across
metrics, runner, CSV export, and tests.
In `@packages/quantum-nematode/quantumnematode/plasticity/snapshot.py`:
- Around line 60-63: The loop that restores state_dicts currently skips snapshot
entries not present in modules (for name, state_dict in snapshot.items()) which
hides partial restores; change it to fail-fast by checking each snapshot key
against modules and raising a clear exception (e.g., ValueError or KeyError)
when name is missing instead of silently continuing, while still calling
modules[name].load_state_dict(deepcopy(state_dict)) for valid keys; ensure the
error message references the missing snapshot key and that this check happens
before any load_state_dict calls.
In
`@packages/quantum-nematode/tests/quantumnematode_tests/utils/test_plasticity.py`:
- Around line 230-255: The test currently only verifies actor weights; expand it
to snapshot and verify critic parameters, optimizer state, and replay buffer (or
training buffer) so restore_brain_state() fully restores training state.
Specifically: capture critic params via snapshot_brain_state(brain)["critic"],
verify they change after modification using brain.critic.parameters(), include
optimizer state by calling brain.optimizer.state_dict() in the snapshot and
compare before/after, and include the replay buffer state (e.g.,
brain.replay_buffer or brain.buffer contents/state) in the snapshot and compare;
after restore_brain_state(brain, snapshot) assert that snapshot["critic"],
snapshot["optimizer"], and snapshot["buffer"] (or the actual buffer attribute)
are equal to the restored ones, and also ensure any training/eval mode flags on
brain are restored if snapshot_brain_state records them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0c4cfffc-8d6f-404a-af80-4b569350fe58
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
configs/studies/plasticity/crh_plasticity.ymlconfigs/studies/plasticity/hybridclassical_plasticity.ymlconfigs/studies/plasticity/hybridquantum_plasticity.ymlconfigs/studies/plasticity/mlpppo_plasticity.ymlconfigs/studies/plasticity/qrh_plasticity.ymldocs/experiments/README.mddocs/experiments/logbooks/008-quantum-brain-evaluation.mddocs/research/quantum-architectures.mddocs/roadmap.mdopenspec/changes/archive/2026-03-21-add-quantum-plasticity-test/.openspec.yamlopenspec/changes/archive/2026-03-21-add-quantum-plasticity-test/design.mdopenspec/changes/archive/2026-03-21-add-quantum-plasticity-test/proposal.mdopenspec/changes/archive/2026-03-21-add-quantum-plasticity-test/specs/experiment-tracking/spec.mdopenspec/changes/archive/2026-03-21-add-quantum-plasticity-test/specs/plasticity-evaluation/spec.mdopenspec/changes/archive/2026-03-21-add-quantum-plasticity-test/tasks.mdopenspec/specs/experiment-tracking/spec.mdopenspec/specs/plasticity-evaluation/spec.mdpackages/quantum-nematode/pyproject.tomlpackages/quantum-nematode/quantumnematode/experiment/__init__.pypackages/quantum-nematode/quantumnematode/experiment/metadata.pypackages/quantum-nematode/quantumnematode/plasticity/__init__.pypackages/quantum-nematode/quantumnematode/plasticity/dtypes.pypackages/quantum-nematode/quantumnematode/plasticity/metrics.pypackages/quantum-nematode/quantumnematode/plasticity/snapshot.pypackages/quantum-nematode/quantumnematode/utils/config_loader.pypackages/quantum-nematode/tests/quantumnematode_tests/utils/test_plasticity.pyscripts/compare_plasticity_results.pyscripts/run_plasticity_test.py
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
scripts/run_plasticity_test.py (1)
443-467:⚠️ Potential issue | 🟠 MajorExport the canonical architecture name, not the raw alias.
The runner normalizes aliases for
BrainType, butbrain_namestays as the YAML value.write_aggregate_csv()then writes that raw label into thearchitecturecolumn, whilecompare_plasticity_results.pymatches exact names likeqrhandcrh. Alias-based configs will train successfully here and then show up as missing data in the comparison step.♻️ Small fix
def run_plasticity_protocol(config: PlasticityConfig) -> list[SeedResult]: """Execute the full plasticity evaluation protocol.""" protocol = config.plasticity - brain_name = config.brain.name + brain_name = config.brain.name @@ from quantumnematode.brain.arch.dtypes import BRAIN_NAME_ALIASES canonical_name = BRAIN_NAME_ALIASES.get(config.brain.name, config.brain.name) brain_type = BrainType(canonical_name) + brain_name = canonical_name🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/run_plasticity_test.py` around lines 443 - 467, The code uses the raw YAML alias in brain_name while canonical_name is computed; update the export so the normalized architecture name is used everywhere: replace or set brain_name to the canonical_name (computed via BRAIN_NAME_ALIASES and BrainType) before results are written (so functions like write_aggregate_csv() receive the canonical name), ensuring the canonical_name/BrainType-derived string is what’s recorded rather than the original config alias.scripts/compare_plasticity_results.py (1)
103-115:⚠️ Potential issue | 🟠 MajorDon't drop earlier CSVs for the same architecture.
This still replaces the first
ArchMetricswith the last one loaded for a givenmetrics.name. If a study is split across multipleaggregate_metrics.csvfiles for the same architecture, the summary and Welch test silently use only the final file.♻️ One way to merge repeated runs
for path_str in args.results: path = Path(path_str) metrics = load_aggregate_csv(path) if metrics: - if metrics.name in all_metrics: - console.print( - f"[yellow]Warning: duplicate architecture '{metrics.name}' " - f"from {path} — overwriting previous entry[/yellow]", - ) - all_metrics[metrics.name] = metrics + existing = all_metrics.get(metrics.name) + if existing is None: + all_metrics[metrics.name] = metrics + else: + existing.bf_values.extend(metrics.bf_values) + existing.ft_values.extend(metrics.ft_values) + existing.pr_values.extend(metrics.pr_values) + if existing.bf_values: + existing.bf_mean = float(np.mean(existing.bf_values)) + existing.bf_std = float(np.std(existing.bf_values)) console.print(f"Loaded: {metrics.name} from {path}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/compare_plasticity_results.py` around lines 103 - 115, The current loop overwrites previous ArchMetrics with the same metrics.name (all_metrics dict keyed to single ArchMetrics), causing earlier CSVs to be dropped; change all_metrics to collect multiple entries per architecture (e.g., dict[str, list[ArchMetrics]] or call a merge function) and update consumers to either merge via a new merge_arch_metrics(existing: ArchMetrics, new: ArchMetrics) helper or to aggregate lists before running summaries/Welch tests; specifically modify the loop that calls load_aggregate_csv and uses metrics.name so that instead of replacing the entry you append the new metrics or merge them (and keep the existing console warning but note multiple sources), and ensure downstream code that reads all_metrics handles the list/merged result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/run_plasticity_test.py`:
- Around line 499-505: The code is forcing DeviceType.CPU when calling
setup_brain_model; instead pass the resolved device from the plasticity config
so requested GPU/QPU backends are honored. Replace the hardcoded DeviceType.CPU
argument to setup_brain_model with the resolved device from the config (e.g.,
use config.device_type or the variable the brain-factory logic computes) and
ensure setup_brain_model(...) receives that device value so the brain
instantiation respects the configured backend.
---
Duplicate comments:
In `@scripts/compare_plasticity_results.py`:
- Around line 103-115: The current loop overwrites previous ArchMetrics with the
same metrics.name (all_metrics dict keyed to single ArchMetrics), causing
earlier CSVs to be dropped; change all_metrics to collect multiple entries per
architecture (e.g., dict[str, list[ArchMetrics]] or call a merge function) and
update consumers to either merge via a new merge_arch_metrics(existing:
ArchMetrics, new: ArchMetrics) helper or to aggregate lists before running
summaries/Welch tests; specifically modify the loop that calls
load_aggregate_csv and uses metrics.name so that instead of replacing the entry
you append the new metrics or merge them (and keep the existing console warning
but note multiple sources), and ensure downstream code that reads all_metrics
handles the list/merged result.
In `@scripts/run_plasticity_test.py`:
- Around line 443-467: The code uses the raw YAML alias in brain_name while
canonical_name is computed; update the export so the normalized architecture
name is used everywhere: replace or set brain_name to the canonical_name
(computed via BRAIN_NAME_ALIASES and BrainType) before results are written (so
functions like write_aggregate_csv() receive the canonical name), ensuring the
canonical_name/BrainType-derived string is what’s recorded rather than the
original config alias.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9211aa3-e479-4f63-9bb6-5f390c838b1c
📒 Files selected for processing (7)
docs/experiments/logbooks/008-quantum-brain-evaluation.mddocs/research/quantum-architectures.mdpackages/quantum-nematode/quantumnematode/plasticity/snapshot.pypackages/quantum-nematode/quantumnematode/utils/config_loader.pypackages/quantum-nematode/tests/quantumnematode_tests/utils/test_plasticity.pyscripts/compare_plasticity_results.pyscripts/run_plasticity_test.py
✅ Files skipped from review due to trivial changes (1)
- docs/research/quantum-architectures.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/quantum-nematode/quantumnematode/plasticity/snapshot.py
- packages/quantum-nematode/quantumnematode/utils/config_loader.py
- docs/experiments/logbooks/008-quantum-brain-evaluation.md
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/run_plasticity_test.py (2)
448-449: Fail fast with explicit required-phase validation.Line 532 indexes
phase_configs[phase_name]directly; misconfigured phase names will throw a lateKeyError. Add an upfront validation with a clear error message.♻️ Proposed refactor
- # Build phase config lookup (by name) - phase_configs: dict[str, PlasticityPhaseConfig] = {p.name: p for p in protocol.phases} + # Build phase config lookup (by name) + phase_configs: dict[str, PlasticityPhaseConfig] = {p.name: p for p in protocol.phases} + required_phase_order = ( + "foraging", + "pursuit_predators", + "thermotaxis_pursuit", + "foraging_return", + ) + missing_phases = [name for name in required_phase_order if name not in phase_configs] + if missing_phases: + raise ValueError( + "Plasticity protocol requires phases " + f"{list(required_phase_order)}; missing: {missing_phases}", + ) @@ - phase_order = ["foraging", "pursuit_predators", "thermotaxis_pursuit", "foraging_return"] + phase_order = list(required_phase_order)Also applies to: 528-533
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/run_plasticity_test.py` around lines 448 - 449, The code builds phase_configs = {p.name: p for p in protocol.phases} but later indexes phase_configs[phase_name] directly which can raise a late KeyError; add explicit upfront validation after creating phase_configs to ensure every required phase_name exists (e.g., iterate over the list of required phase names used later or values from protocol references) and raise a clear ValueError with a descriptive message if any are missing; update references where phase_configs[phase_name] is used (the lookup sites around lines 528–533 and 532) to rely on the validated mapping so no KeyError occurs at runtime.
74-75: Standardize function docstrings to NumPy style.New/updated function docstrings are currently short-form; align them to the project’s NumPy-style docstring standard for consistency.
As per coding guidelines,
**/*.py: Include NumPy-style docstrings for documentation.Also applies to: 111-112, 165-169, 225-233, 274-275, 326-327, 402-403, 444-445, 588-588
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/run_plasticity_test.py` around lines 74 - 75, Several function docstrings (including the one above the env = create_env_from_config call and the functions spanning lines 111-112, 165-169, 225-233, 274-275, 326-327, 402-403, 444-445, and 588) use short-form or non-NumPy style docstrings; update each to the project's NumPy-style format by adding a one-line summary, a short description if needed, and explicit sections for Parameters (with types and descriptions), Returns (with types and descriptions) and Raises (if applicable) using the function names near those diffs (e.g., the constructor that "Construct a fresh agent for a phase, reusing the existing brain", the functions around the other ranges) so each docstring follows the consistent NumPy convention expected by the linter and docs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/run_plasticity_test.py`:
- Around line 448-449: The code builds phase_configs = {p.name: p for p in
protocol.phases} but later indexes phase_configs[phase_name] directly which can
raise a late KeyError; add explicit upfront validation after creating
phase_configs to ensure every required phase_name exists (e.g., iterate over the
list of required phase names used later or values from protocol references) and
raise a clear ValueError with a descriptive message if any are missing; update
references where phase_configs[phase_name] is used (the lookup sites around
lines 528–533 and 532) to rely on the validated mapping so no KeyError occurs at
runtime.
- Around line 74-75: Several function docstrings (including the one above the
env = create_env_from_config call and the functions spanning lines 111-112,
165-169, 225-233, 274-275, 326-327, 402-403, 444-445, and 588) use short-form or
non-NumPy style docstrings; update each to the project's NumPy-style format by
adding a one-line summary, a short description if needed, and explicit sections
for Parameters (with types and descriptions), Returns (with types and
descriptions) and Raises (if applicable) using the function names near those
diffs (e.g., the constructor that "Construct a fresh agent for a phase, reusing
the existing brain", the functions around the other ranges) so each docstring
follows the consistent NumPy convention expected by the linter and docs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9805824e-600c-4407-b9be-ef412aa3b3fa
📒 Files selected for processing (1)
scripts/run_plasticity_test.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New code
quantumnematode/plasticity/— Package module with dtypes, metrics (BF/FT/PR computation, convergence detection), and brain state snapshot/restorescripts/run_plasticity_test.py— Sequential training protocol with eval matrix, CSV export, Rich console summaryscripts/compare_plasticity_results.py— Post-hoc cross-architecture comparison with t-testconfigs/studies/plasticity/— 5 architecture configs (QRH, CRH, HybridQuantum, HybridClassical, MLP PPO)PlasticityConfigPydantic models in config_loader.pyPlasticityMetadatamodels in experiment/metadata.pyDocumentation updates
docs/experiments/logbooks/008-quantum-brain-evaluation.md— Status → complete, QA-7 results section addeddocs/research/quantum-architectures.md— QA-6 deferred, QA-7 completed, strategic assessment section, updated priority table and decision gatesdocs/roadmap.md— Phase 2 quantum evaluation status notedocs/experiments/README.md— Logbook 008 marked completedTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Public API
Documentation
Tests
Chores