Repository navigation
fiber_trace: don't require the monorepo lasagna package just to decod… - #2090
Merged
hendrikschilling merged 2 commits intoOct 22, 2026
Merged
hendrikschilling merged 2 commits into
hendrikschilling merged 2 commits into
Conversation
…e normals vesuvius.neural_tracing.fiber_trace.geometry imported _decode_normals from lasagna.omezarr_pyramid (or a bare omezarr_pyramid on PYTHONPATH). lasagna is not a declared dependency of vesuvius, so with a normal 'uv sync --extra models' importing vesuvius.neural_tracing.fiber_trace (and collecting tests/neural_tracing/test_fiber_trace.py) failed with ModuleNotFoundError. Keep preferring lasagna's implementation, but fall back to an identical local 4-line decoder. Adds a byte-for-byte parity test against lasagna/omezarr_pyramid.py and a test that the module imports with lasagna blocked.
|
@sfc0324 is attempting to deploy a commit to the scroll Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Looks mostly fine, but this PR duplicates the lasagna normal decoder in vesuvius. To avoid duplication please move it into vesuvius instead and make lasagna code use this shared decoder instead. |
Move Lasagna's uint8 (nx, ny) hemisphere decoder into vesuvius.neural_tracing.normals.decode_normals and call it from fiber_trace, so decoding normals no longer imports lasagna. lasagna.omezarr_pyramid re-exports that function as _decode_normals, keeping the existing name for callers such as the pyramid moment pool and fiber_trace_2d. Co-authored-by: sfc0324 <sfc0324@users.noreply.github.com>
Contributor
Author
|
Thanks for the review! I moved the normal decoder into vesuvius ( |
hendrikschilling
enabled auto-merge (squash)
October 22, 2026 15:36
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.
In one sentence:
vesuvius.neural_tracing.fiber_tracecan be imported from a normalvesuviusinstall without the monorepolasagnapackage onPYTHONPATH.One real example: Starting with a fresh
uv sync --extra modelsofvesuviusata4979b9, I rantests/neural_tracing/test_fiber_trace.py, and it produced 52 passing tests instead of a collection-timeModuleNotFoundError: No module named 'lasagna'.Before:
fiber_trace/geometry.pyimports_decode_normalsfromlasagna.omezarr_pyramid, falling back to a bareomezarr_pyramidmodule.lasagnais not a declared dependency ofvesuvius, so importingvesuvius.neural_tracing.fiber_trace(and collecting its test file) fails unless the user manually putsvilla/lasagnaonPYTHONPATH. Only this 4-line decoder is used from lasagna here.After this PR: lasagna's implementation is still preferred when importable; otherwise an identical local
_decode_normals_u8is used. Behaviour with lasagna installed is unchanged.Proof: Same environment, base
a4979b9vs this branch.Before (
a4979b9):After (this PR;
test_fiber_trace.py+ newtest_fiber_trace_decode_normals.py):The 3 new tests: known values (128→0, 255→1, 1→−1, float32), byte-for-byte parity with
lasagna/omezarr_pyramid.py::_decode_normalson random uint8 input (loaded from the monorepo file; skips if absent), and import withlasagna/omezarr_pyramidblocked insys.modules.Why / where this is useful: People who
pip/uvinstallvesuviusto train or run the fiber tracer, and CI/test runs of thevesuviuspackage on its own, no longer need the undocumentedPYTHONPATH=lasagnastep for this module.Details
a4979b9; this branch = one commit on top.which g++ c++finds nothing), Python 3.14.8, torch 2.12.1+cu130, uv 0.13.0. Install:cd vesuvius && uv sync --extra models --no-install-package volume-cartographer(volume-cartographer skipped because it needs a C++ toolchain to build).vesuvius/):uv run --with pytest python -m pytest -q -p no:cacheprovider tests/neural_tracing/test_fiber_trace.py tests/neural_tracing/test_fiber_trace_decode_normals.pysrc/vesuvius/neural_tracing/fiber_trace/geometry.py(local decoder + third import fallback); newtests/neural_tracing/test_fiber_trace_decode_normals.py.fiber_trace_2dandfiber_trace_3duse much more of lasagna (tiled_predict3d,live_omezarr_cache, ...) and still require it; their test files still error at collection without lasagna. Keeping a copy of the decoder means the two must stay in sync — the parity test guards that.