Repository navigation
Restore global fiber workflows and native GPU/chunk tracing - #2231
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba5e34e4de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ob=s['u_offsets'][s['v_offset_b'][edge]] | ||
| hu=torch.as_tensor(np.where(reverse,ob,oa),dtype=torch.float64) | ||
| vu=torch.as_tensor(np.where(reverse,oa,ob),dtype=torch.float64) | ||
| c=candidates(state[h],state[v],torch.as_tensor(dh),torch.as_tensor(dv),widths,sigma_h,hu,vu,**sigma_args) |
There was a problem hiding this comment.
Preserve shared geometry when applying U offsets
When a checkpoint combines u_offsets with shared_curve_geometry or shared_curve_hypothesis_mapping, the initial call correctly evaluates the event-specific sh/sv geometry, but this second call restarts from raw state[h]/state[v]. It therefore drops the saved shared-curve positions while computing the corrected matches, producing incorrect match counts and U/V loss metrics for deformed shared-curve checkpoints; pass the already-prepared sh and sv here.
Useful? React with 👍 / 👎.
| l1=float(error.mean()),correct_percent=float(100*correct.mean()),median=float(np.median(error)), | ||
| p95=float(np.quantile(error,.95)),max=float(error.max()),sign_violations=int((signed&(order<0)).sum()), | ||
| signed_events=int(signed.sum()),sign_violation_percent=float(100*(signed&(order<0)).sum()/signed.sum())) |
There was a problem hiding this comment.
Handle empty enabled cohorts in the inpainting report
If a solve rejects every event in either class, or the dataset contains no parallel or perpendicular events, error is empty and np.quantile(error, .95)/error.max() raises after the optimization has completed. Rejection is a valid model outcome, so this reporting path should emit None metrics for empty cohorts (and guard the zero signed-event denominator) rather than losing the completed workflow.
Useful? React with 👍 / 👎.
| manifest=root/'download_manifest.json';previous={} | ||
| if manifest.exists():previous={entry['key']:entry for entry in json.loads(manifest.read_text())['objects']} |
There was a problem hiding this comment.
Remove stale objects when refreshing a reference prefix
When an existing output_root is refreshed after an object is removed from the S3 prefix—or is reused for a different prefix—the old local file is never rejected or deleted, while the new manifest simply omits it. A required file missing from the new source can therefore be silently supplied by the previous download, yielding a mixed reference surface whose on-disk contents do not match the recorded source; verify the manifest source and prune or reject local files absent from the current listing.
Useful? React with 👍 / 👎.
| hashes=validate_sources(cfg,doc) | ||
| path=Path(settings['output_root'])/item['label'].lower() | ||
| meta=json.loads((path/'meta.json').read_text());stats=json.loads((path/'surface_statistics.json').read_text()) | ||
| assert meta['scale']==[1/settings['grid_step']]*2 |
There was a problem hiding this comment.
Honor the surface exporter's default grid step in audits
construct() treats grid_step as optional and defaults it to 100, so a surface can be successfully generated from a valid configuration that omits this setting. Auditing that same configuration then raises KeyError here instead of validating the artifact; use the same default as the exporter or read the recorded step from the surface statistics.
Useful? React with 👍 / 👎.
In one sentence: Restore reusable global fiber-layout workflows and native CPU/GPU chunk tracing, including their evaluation, cache and prediction-pooling interfaces.
One real example: Re-evaluating supplied saved fiber-layout checkpoints with the restored tools reproduced their scores and all recorded objective terms, without rerunning optimization.
Before: Global-layout workflows were missing from the extracted package, and native tracing recovery was being reviewed separately in #2233.
After this PR: Shared CLIs and Python/native APIs expose both sets of functionality. Stage choices, dataset identity, projection scale and experiment settings are explicit inputs. Documentation contains minimal usage examples, not historical runs or tuned configurations. This PR incorporates the complete branch history of #2233; that PR is superseded by this combined review.
Proof: On the combined branch: 966 Python tests passed (9 skipped, one unchanged socket-dependent test deselected), plus three real-checkpoint checks including full-objective recomputation. The rebuilt native chunk/reverse-consistency/pooling tests passed 24 cases; native Python API tests passed 16 with one opt-in real-data test skipped. No fresh GPU solve or CUDA validation was run for this merge; #2233 records the original GPU and real-data tracing validation.
Why / where this is useful: Enables reproducible fiber-layout experiments through shared implementations rather than private experiment scripts.
Parts included
1. Global fiber-layout workflows (
vesuvius)vesuvius/docs/fiber_workflows.mdandvesuvius/docs/fiber_layout_benchmark.md.2. Native CPU/GPU chunk tracing (incorporated from #2233)
volume-cartographer/docs/fiber_chunk_tracing.mdandvolume-cartographer/docs/trace_recovery.md.3. Prediction pooling and tracing evaluation integration
Merge verification
Current main was merged first. Conflicts were resolved in the archive ignore rules and benchmark documentation, retaining the generic docs and local archive exclusions. The #2233 branch then merged cleanly. The final integration cleanup changes documentation/test descriptions only, not numerical code.
Read-only checkpoint verification uses
FIBER_LAYOUT_REFERENCE_SPECwithvesuvius/tests/test_fiber_workflows_real_data.py. Native merge verification used the existing Linux RelWithDebInfo CPU build; macOS/arm64 and CUDA were not rerun here. CPU/GPU bit identity is not claimed.Automated verification was performed by Codex; the personal-verification checkbox is left for human review.