docs: record h2 output-cloud geometry boundary#308
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the H2 output-cloud geometry candidate signal, adding a new evaluation script, unit tests, and extensive documentation updates across the roadmap and evidence logs. The changes include several result artifacts and update the project's operating state to reflect this new research-side candidate. Feedback for the new script focuses on improving robustness by adding validation to ensure at least two timesteps are present to prevent runtime errors and implementing safer dictionary access when parsing summary JSON files to handle potential null values.
| if responses_f32.shape[2] < 2: | ||
| raise ValueError("output-cloud geometry needs at least two repeats per timestep") | ||
|
|
||
| sample_count, timestep_count, repeat_count = responses_f32.shape[:3] |
There was a problem hiding this comment.
The function should validate that timestep_count is at least 2. If a cache with only one timestep is provided, the script will crash at line 92 when attempting to np.stack an empty list of centroid_pairs (since the nested loops starting at line 85 will not find any pairs). Additionally, features like within_timestep_pair_rmse_slope require at least two points to be meaningful.
| sample_count, timestep_count, repeat_count = responses_f32.shape[:3] | |
| sample_count, timestep_count, repeat_count = responses_f32.shape[:3] | |
| if timestep_count < 2: | |
| raise ValueError("output-cloud geometry needs at least two timesteps") |
| raw_h2_metrics = summary.get("raw_h2", {}).get("logistic", {}).get("aggregate_metrics") | ||
| lowpass_h2_metrics = summary.get("lowpass_h2", {}).get("logistic", {}).get("aggregate_metrics") |
There was a problem hiding this comment.
The current chaining of .get() calls is susceptible to an AttributeError if any intermediate key (like "raw_h2" or "logistic") exists in the JSON but has a null value. In Python, dict.get(key, default) returns None if the key is present with a null value, which causes the subsequent .get() call to fail. Using the (summary.get("key") or {}) pattern provides better robustness against such cases.
| raw_h2_metrics = summary.get("raw_h2", {}).get("logistic", {}).get("aggregate_metrics") | |
| lowpass_h2_metrics = summary.get("lowpass_h2", {}).get("logistic", {}).get("aggregate_metrics") | |
| raw_h2_metrics = (summary.get("raw_h2") or {}).get("logistic", {}).get("aggregate_metrics") | |
| lowpass_h2_metrics = (summary.get("lowpass_h2") or {}).get("logistic", {}).get("aggregate_metrics") |
Summary
Verification
C:\Users\Ding\miniforge3\envs\diffaudit-research\python.exe -X utf8 -m pytest tests/test_review_h2_output_cloud_geometry_script.py -qC:\Users\Ding\miniforge3\envs\diffaudit-research\python.exe -X utf8 scripts/check_markdown_links.pyC:\Users\Ding\miniforge3\envs\diffaudit-research\python.exe -X utf8 scripts/check_public_surface.pyC:\Users\Ding\miniforge3\envs\diffaudit-research\python.exe -X utf8 scripts/export_admitted_evidence_bundle.py --checkC:\Users\Ding\miniforge3\envs\diffaudit-research\python.exe -X utf8 scripts/run_pr_checks.py