Refactor: break the 3 module couplings blocking a clean forward/inverse split#106
Open
joglekara wants to merge 1 commit into
Open
Refactor: break the 3 module couplings blocking a clean forward/inverse split#106joglekara wants to merge 1 commit into
joglekara wants to merge 1 commit into
Conversation
…se split Prerequisite, behavior-preserving moves toward #105. These break the only dependency cycles/wrong-direction edges in the package, so the forward kernel can later be isolated (as extras or a separate repo) without further surgery. - Merge `utils/data_handling` + `utils/process` into one `tsadar/data/` package. They were mutually recursive (data_handling->process and process->data_handling); the split was artificial since preprocessing *is* data handling. Now the cycle is intra-package. - Move `postprocess.py` into `tsadar/inverse/`. It is post-*fit* processing and imported `inverse.loss_function`/`inverse.loops` while `inverse.fitter` imported it back — a true `inverse <-> process` cycle. Now intra-`inverse`. - Move the interactive runner `calc_vs_data.py` from `forward/` to `inverse/`. It imported `inverse.fitter`/`inverse.loss_function`, making `forward` depend on `inverse` (wrong direction). `forward` no longer imports `inverse`. No behavior change: pure file moves + import-path updates (incl. tests). Verified statically — all intra-tsadar imports resolve, every module compiles, and forward->inverse / data->inverse / data->forward edges are gone; `core` kernel remains free of inverse/forward/data/plotting deps. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This was referenced Jun 17, 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
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.
What
Behavior-preserving refactor that breaks the three module couplings identified in #105. These are the prerequisites for isolating a clean, light-dependency forward kernel — they're required regardless of whether we ultimately pick Option A (1 repo + install extras) or Option B (2 repos), so they don't block on that decision.
Why
The package had two true dependency cycles and one wrong-direction edge. Separate packages can't have circular deps, so these had to go before any split — and they're worth fixing even within one package:
utils/data_handling↔utils/processtsadar/data/package (cycle is now intra-package)inverse↔utils/processpostprocessimportedinverse.{loss_function,loops};fitterimportedpostprocessbackpostprocessmoved intoinverse/; now intra-inverseforward→inversecalc_vs_dataimportedinverse.{fitter,loss_function}calc_vs_datamoved toinverse/;forwardno longer importsinverseChanges
utils/data_handling+utils/process→tsadar/data/. The split was artificial — preprocessing is data handling, and the two were mutually recursive.utils/process/postprocess.py→tsadar/inverse/postprocess.py. It's post-fit processing; it belongs with the inverse code that produces the fit.forward/calc_vs_data.py→inverse/calc_vs_data.py. The interactive "fit forward vs. data" runner sits naturally beside the fitter.tsadar/and the test suite. No logic changes.Verification
The local env has a pre-existing, unrelated jax version mismatch (
sph_harm_y), so this was validated statically:tsadarimports resolve to existing files/symbols.py_compileon every trackedtsadar/+tests/file passes.forward→inverse,data→inverse,data→forwardedges are gone; thecorekernel still imports nothing frominverse/forward/data/plotting.A reviewer with a working jax should confirm
import tsadarand the test suite still pass.Not in scope
The A vs B packaging decision (#105) and the
pyproject.tomlextras / dependency-gating — those follow once the boundaries are settled.Toward #105.
🤖 Generated with Claude Code