Spiral workspace ergo - #1287
Merged
Merged
Conversation
memory use is not static , causes huge allocs that take forever to recover
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Spiral workflow across the Python fitter, its dependency model, and the VC3D client integration by introducing a validated configuration schema, adding structured progress reporting, and reworking several performance/VRAM paths (notably patch sampling and sparse volume access). It also adds checkpoint migration/export utilities and updates docs for dataset distribution.
Changes:
- Introduce a validated
Configschema + catalog/presets and wire it through fitter/service/client. - Add structured progress reporting and extend VC3D to consume richer status/configuration + artifact transfer progress.
- Rework performance-sensitive data paths (host-resident patch atlas gathers, resident brick pools, track-walk gating) and add new regression/perf tests.
Reviewed changes
Copilot reviewed 75 out of 76 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| volume-cartographer/uv.lock | Locks new/updated Python deps (incl. rustworkx; platform marker for posix-ipc). |
| volume-cartographer/scripts/spiral/update_checkpoint.py | New CLI to migrate legacy checkpoints’ embedded configs to current schema. |
| volume-cartographer/scripts/spiral/transforms.py | Schema key renames; shared-leaf transform tensors; flow field construction tweaks. |
| volume-cartographer/scripts/spiral/tests/test_vram_reductions.py | Expanded VRAM/grad-streaming coverage incl. cylindrical flow + shared leaves + host atlas tests. |
| volume-cartographer/scripts/spiral/tests/test_update_checkpoint.py | New tests for legacy->current config migration and checkpoint updating. |
| volume-cartographer/scripts/spiral/tests/test_track_walk.py | Updates track-walk defaults/validation; skips superseded native tests. |
| volume-cartographer/scripts/spiral/tests/test_track_graph_real.py | New “real data” gate test for track graph + native walk behavior. |
| volume-cartographer/scripts/spiral/tests/test_track_crossing_cache.py | Updates track sampling key names (new schema). |
| volume-cartographer/scripts/spiral/tests/test_spiral_progress.py | New unit tests for the progress reporter. |
| volume-cartographer/scripts/spiral/tests/test_spiral_helpers.py | Update config key names (new schema). |
| volume-cartographer/scripts/spiral/tests/test_spiral_headless.py | Adds progress payload wiring + interactive horizon behavior tests + renamed config keys. |
| volume-cartographer/scripts/spiral/tests/test_speedup_equivalence.py | Adjusts flow field ctor calls for new API. |
| volume-cartographer/scripts/spiral/tests/test_sparse_cuda_cache.py | Migrates tests from bounded LRU cache to resident brick pool approach. |
| volume-cartographer/scripts/spiral/tests/test_sdt_losses.py | Updates dense-loss sampling keys to new schema names. |
| volume-cartographer/scripts/spiral/tests/test_phase_spacing.py | Removes optional-input selection plumbing; updates keys; adjusts validation expectations. |
| volume-cartographer/scripts/spiral/tests/test_pcl_sampling.py | Renames PCL sampling config keys. |
| volume-cartographer/scripts/spiral/tests/test_lr_schedule.py | New tests for LR schedule realignment and per-parameter-group scaling. |
| volume-cartographer/scripts/spiral/tests/test_interactive_influence.py | Updates config key names; flow-stage key rename coverage. |
| volume-cartographer/scripts/spiral/tests/test_flatten_spiral_checkpoint.py | New tests for flatten checkpoint exporter helpers. |
| volume-cartographer/scripts/spiral/tests/test_config.py | New tests for Config catalog completeness + validation behavior. |
| volume-cartographer/scripts/spiral/strip_paths.py | Adds connectivity filtering + deterministic jitter for shortest-path variety. |
| volume-cartographer/scripts/spiral/spiral_progress.py | New dependency-free progress reporter with snapshot/publish/console support. |
| volume-cartographer/scripts/spiral/spiral_helpers.py | Wires progress into exports; updates many config keys (new schema). |
| volume-cartographer/scripts/spiral/sdt_losses.py | Updates dense-loss sampling keys to new schema names. |
| volume-cartographer/scripts/spiral/satisfaction_metrics.py | Adds progress stages for satisfaction evaluation steps. |
| volume-cartographer/scripts/spiral/README.md | Adds flattening docs; updates sparse-volume approach documentation; progress notes. |
| volume-cartographer/scripts/spiral/pyproject.toml | Adds rustworkx and platform marker for posix-ipc. |
| volume-cartographer/scripts/spiral/prefetch.py | Updates docstring to reflect host-resident patch atlas gather. |
| volume-cartographer/scripts/spiral/phase_tuning.py | Switch to new Config defaults + renamed keys. |
| volume-cartographer/scripts/spiral/losses.py | Moves patch atlas bilinear gather to batch-build time; renames keys; reduces GPU work. |
| volume-cartographer/scripts/spiral/lasagna_data.py | Loads resident brick pools from sidecars; adds progress callbacks; sidecar requirement checks. |
| volume-cartographer/scripts/spiral/influence.py | Renames influence keys; adjusts flow displacement bound semantics. |
| volume-cartographer/scripts/spiral/flow_triton.py | Adds config-driven direct-LR enablement override logic. |
| volume-cartographer/scripts/spiral/flow_fields.py | Refactors flow-field scaling/direct-LR handling; adds cylindrical pending-graph accumulation. |
| volume-cartographer/scripts/spiral/fit_session.py | Moves run-mutable config selection to Config catalog; API version bump; validation simplification. |
| volume-cartographer/scripts/spiral/find_inconsistent_windings.py | Switch to Config defaults + renamed keys. |
| volume-cartographer/scripts/spiral/dt_targets.py | Moves patch-atlas gather to host-resident path; uploads only interpolated points. |
| volume-cartographer/scripts/spiral/ddp_helpers.py | Renames distributed split config key. |
| volume-cartographer/scripts/spiral/configs/track_walk.json | New preset for track-walk configuration. |
| volume-cartographer/scripts/spiral/config.py | New validated Config schema + catalog/presets + runtime-impact metadata. |
| volume-cartographer/scripts/spiral/calibration/gradnorm.py | Switch to Config defaults + renamed keys. |
| volume-cartographer/scripts/spiral/benchmark_rustworkx_track_graph.py | New benchmark script for rustworkx-based track graph representation. |
| volume-cartographer/python/CMakeLists.txt | Adds -O3 for vc_track_crossings in RelWithDebInfo (GNU/Clang). |
| volume-cartographer/core/test/test_spiral_config_profile_editor.cpp | New Qt test for Spiral advanced config profile editor grouping/search/responsiveness. |
| volume-cartographer/core/test/CMakeLists.txt | Adds new core Qt test target. |
| volume-cartographer/apps/VC3D/VCAppMain.cpp | Adds wheel focus filter + raises Qt image allocation limit at runtime. |
| volume-cartographer/apps/VC3D/test/test_spiral_reload_comparison.cpp | New test for reload normalization behavior. |
| volume-cartographer/apps/VC3D/test/CMakeLists.txt | Adds new VC3D test executable + ctest entry. |
| volume-cartographer/apps/VC3D/SpiralServiceManager.hpp | Adds configuration catalog/review signals; runIterations inputs param; transfer progress signals. |
| volume-cartographer/apps/VC3D/SpiralServiceManager.cpp | API v15 support; config catalog fetch; run plan/confirmation flow; checkpoint download progress. |
| volume-cartographer/apps/VC3D/SpiralReloadComparison.hpp | Adds support for ignoring run-mutable paths during reload comparison. |
| volume-cartographer/apps/VC3D/SpiralPanel.hpp | Removes optional input UI; adds preview/checkpoint progress UI plumbing. |
| volume-cartographer/apps/VC3D/SpiralArtifactCache.hpp | Adds artifact fetch progress signal. |
| volume-cartographer/apps/VC3D/SpiralArtifactCache.cpp | Tracks file/byte totals; streams SHA-256; emits detailed fetch progress. |
| volume-cartographer/apps/VC3D/elements/SpiralConfigProfileEditor.hpp | Adds grouped control UI, search, catalog-driven editors, and pop-out window support. |
| volume-cartographer/apps/VC3D/CMakeLists.txt | Installs Spiral JSON presets alongside Python scripts. |
| scrollprize.org/docs/38_tutorial_spiral.md | Updates dataset source + download command (now data server + rclone). |
| scrollprize.org/docs/02_data_datasets.md | Updates dataset browse link to data server. |
| lasagna/tests/test_opt_loss_flatten.py | Adds coverage for exporting flatten-map sidecar and omitting model file. |
| lasagna/model.py | Adjusts flatten invert chunking behavior (removes forced chunk floor). |
| lasagna/fit2tifxyz.py | Adds --omit-model and --flatten-map-output options; threads into exports. |
| lasagna/fit.py | Adds lifecycle callback hooks for “saving” stages; main accepts lifecycle_fn. |
| lasagna/fit_service.py | Uses lifecycle hook; adds omit-model/flatten-map export options; improves stage naming. |
Comments suppressed due to low confidence (1)
volume-cartographer/scripts/spiral/losses.py:368
patch_atlasdefaults to None, but the new implementation unconditionally callspatch_atlas.lookup(...), which will raise an AttributeError. Either makepatch_atlasrequired (no default) or fail fast with a clearer error.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self._event_dim = event_dim | ||
| self._flow_range_zyx = self.flow_max_corner_zyx - self.flow_min_corner_zyx | ||
| self.num_flow_timesteps = getattr(flow_field, 'num_flow_timesteps', 1) | ||
| self.num_flow_timesteps = getattr(flow_field, 'model_num_flow_timesteps', 1) |
| cached = self._pinned_scaled_logits | ||
| if cached is None or (needs_grad and not cached.requires_grad): | ||
| # Pin the 0th logit (i.e. theta=0 on 1th winding) to be zero, to avoid a jump going from winding #0 to #1 | ||
| # Pin the 0th logit (i.e. theta=0 on 1th winding) to be zero, to avoid a jump going from winding #0 to #1. |
Comment on lines
45
to
+49
| # kills the F.interpolate forward+backward and its full-resolution | ||
| # transient. Off by default because it slightly changes the effective | ||
| # interpolant. | ||
| return os.environ.get('FIT_SPIRAL_DIRECT_LR', '0') == '1' | ||
| # interpolant. Normally set via the model_flow_field_direct_lr config key | ||
| # (threaded through as `default`); FIT_SPIRAL_DIRECT_LR, when set, | ||
| # overrides the config. |
Comment on lines
111
to
115
| ```bash | ||
| uvx --from huggingface_hub hf buckets sync \ | ||
| hf://buckets/scrollprize/datasets/spiral/PHercParis4 \ | ||
| ./spiral-dataset/PHercParis4 | ||
| rclone copy :http: /home/sean/Desktop/PHercParis4 \ | ||
| --http-url https://dl.ash2txt.org/datasets/spiral_datasets/PHercParis4/ \ | ||
| --transfers 32 -P | ||
| ``` |
Comment on lines
+11
to
+25
| DEFAULT_TRACK_ROOT = Path( | ||
| "/home/sean/Desktop/spiral_dataset/to_hf/tracks") | ||
| TRACK_BASENAME = "2um_ds2_ps256_surf_v2.dbm" | ||
| KNOWN_CYCLE_SOURCE_ID = 8065948596289 | ||
|
|
||
|
|
||
| class RealTrackGraphTests(unittest.TestCase): | ||
| @classmethod | ||
| def setUpClass(cls): | ||
| root = Path(os.environ.get( | ||
| "VC_SPIRAL_REAL_TRACKS", DEFAULT_TRACK_ROOT)) | ||
| cache_path = root / f"{TRACK_BASENAME}.crossings.npz" | ||
| if not cache_path.is_file(): | ||
| raise unittest.SkipTest( | ||
| f"real 2um crossing cache is unavailable: {cache_path}") |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 74 out of 75 changed files in this pull request and generated no new comments.
Suppressed comments (2)
volume-cartographer/scripts/spiral/transforms.py:34
- IntegratedFlowDiffeomorphism is reading
model_num_flow_timestepsoff the flow field, but the flow field classes exposenum_flow_timesteps(see flow_fields.py). With the current code, any multi-timestep flow will silently fall back to 1 and take the time-invariant fast path, producing incorrect integration behavior.
volume-cartographer/scripts/spiral/spiral_progress.py:182 - _emit() reads/updates
_last_publishand_last_consolewithout holding_lock. Sinceupdate()can be called from worker threads, this creates a data race that can break the intended rate-limiting (duplicate publishes/console writes) and undermines the “thread safe” guarantee in tests/docs.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 74 out of 75 changed files in this pull request and generated no new comments.
Suppressed comments (4)
volume-cartographer/scripts/spiral/spiral_progress.py:218
_heartbeat_main()reads_last_consoleunder the lock but then writes_last_consoleoutside it. That’s still a race with_emit(). Move the_last_consoleupdate under the lock (and ideally decide whether to emit while holding the lock) to keep rate limiting coherent across threads.
volume-cartographer/scripts/spiral/benchmark_rustworkx_track_graph.py:23memory_bytes()unconditionally reads/proc/self/status, which will fail on non-Linux platforms (e.g. macOS) where this benchmark may still be useful. Make RSS reporting best-effort with a portable fallback (e.g.resource.getrusage) so the script doesn’t crash outside Linux.
def memory_bytes():
"""Return current RSS and peak RSS on Linux."""
values = {}
with open("/proc/self/status", encoding="ascii") as stream:
for line in stream:
key, value = line.split(":", 1)
if key in {"VmRSS", "VmHWM"}:
values[key] = int(value.split()[0]) * 1024
return values["VmRSS"], values["VmHWM"]
volume-cartographer/scripts/spiral/spiral_progress.py:182
ProgressReporterupdates_last_publish/_last_consolewithout holding_lock, but_heartbeat_main()reads/writes_last_consoleconcurrently. This creates a data race and can break the “thread safe” guarantee (e.g. duplicated/extra console emits). Guard the rate-limit state updates with the same lock used for_active.
This issue also appears on line 214 of the same file.
volume-cartographer/scripts/spiral/config.py:157
model_flow_field_direct_lrdefaults toTruehere, butflow_triton.direct_lr_enabled()documents this mode as “Off by default because it slightly changes the effective interpolant” (flow_triton.py:46-52). Please reconcile these defaults/docs so the workspace default is unambiguous.
self.model_flow_field_high_res_lr_scale_initial = 0.2
self.model_flow_field_high_res_lr_scale_final = 0.2
self.model_flow_field_high_res_lr_ramp_start_step = 0
self.model_flow_field_high_res_lr_ramp_steps = 1
self.model_flow_field_direct_lr = True
skyward7187
approved these changes
Jul 31, 2026
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
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.
No description provided.