Skip to content

Refactor: isolate a standalone forward kernel — 2 repos vs. 1 repo with 2 install paths? #105

Description

@joglekara

Context

The package has grown to mix three concerns that have different audiences and dependency footprints:

  1. Forward modeling — turning plasma parameters into a Thomson spectrum (physics → synthetic diagnostic).
  2. Fitting / inverse — optimization loops, loss functions, data preprocessing, postprocessing.
  3. Orchestration — deck/config processing, MLflow runners, plotting.

Two external pulls motivate cleaning this up:

  • A 3rd-party request to use the forward solve standalone and let the user decide how the fit is performed (bring-your-own optimizer). This is the strongest argument for isolating a clean forward kernel.
  • An internal preference (from the primary user) to decompose into well-defined modules: deck/runners, postprocessor, inverse, data handler, interactive runner, forward module, synthetic diagnostic, and physics.

This issue records an analysis of the current structure and proposes a path. It ends with one decision to make (see bottom).

Findings (from the import graph)

The codebase (~9k LOC) is already close to a clean layered shape. Two facts dominate the design:

1. core/ is already a clean, IO-free forward kernel ✅

Everything under core/ (physics, EDF/params, synthetic diagnostic) imports only jax, scipy, numpy, interpax, equinox, plus one internal helper (utils.vector_tools). It pulls in none of the heavy/dirty deps — no mlflow, optax, opencv, skimage, boto3, pandas, xlrd. Those are confined to the fitting/data/IO/plotting layers.

This is exactly the boundary the 3rd party wants: import tsadar.core... gives a differentiable forward model with light deps and no opinion about how you fit.

2. Three couplings are the actual "muddle" 🔧

These must be resolved for any split (separate packages cannot have circular dependencies) — and they're worth fixing even if we keep one package:

  • inverseutils/process (true cycle): inverse/fitter.py imports process.postprocess; process/postprocess.py imports back into inverse.loss_function and inverse.loops.
  • utils/data_handlingutils/process (true cycle): data_handling/load_ts_data.pyprocess.warpcorr, while process/evaluate_background.py and process/prepare.pydata_handling.
  • forwardinverse (wrong-direction edge): forward/calc_vs_data.py (the "interactive runner") imports inverse.fitter and inverse.loss_function. Forward should not depend on inverse.

Recommendation

Modules ≠ repos. The 8-module decomposition is an excellent package decomposition. Splitting it into 8 repos would multiply versioning/CI/release/pinning overhead 8× for a ~9k-LOC codebase whose layers co-evolve — and it isn't even possible today because of the two dependency cycles above.

What we actually want — "use the forward solve standalone, bring your own fit" — is a dependency-isolation problem, solved by a clean layered DAG plus dependency gating.

Target layering (strict acyclic, bottom → top)

The 8 desired modules map cleanly onto this:

utils            (vector_tools, misc)                    ← leaf
physics          (form_factor, generate_spectra, chi)    "Physics"          ─┐
params/edf       (ts_params, distribution_functions)     "EDF"              │ LIGHT-DEP
diagnostic       (thomson_diagnostic, irf)               "synthetic diag"   ─┘ KERNEL
────────────────────────────────────────────────────────────────────────────
data             (load, calibration, lineouts, bg,       "data handler"
                  throughput, warp, feature, visualizer)
plotting         (plotters, lineout_plot, edf_movie)     "postprocessor" primitives
forward          (calc_series = forward + series runner) "forward module"
inverse          (loss, loops, fitter, postprocess)      "inverse"
interactive      (calc_vs_data)                          "interactive runner"
runner/deck      (config, mlflow orchestration)          "tsadar"

The kernel (utils → physics → params/edf → diagnostic) is the standalone forward solver. Everything above it is fitting/data/orchestration.

Prerequisite refactors (needed regardless of repo count)

  • Move postprocess.py into inverse/ — it's post-fit processing. Turns the inverse↔process cycle into an intra-package call. (There's already a stray empty tsadar/inverse/process/ dir suggesting this was started.)
  • Merge data_handling + process into one data package — they're mutually recursive because the split is artificial; preprocessing is data handling.
  • Relocate calc_vs_data (interactive runner) beside inverse (or push shared helpers down: load_data_for_fittingdata, keep LossFunction in inverse) so forward no longer imports inverse.
  • Note: "postprocessor = plotting" conflates two things. Plotting primitives are a clean leaf; the post-fit orchestration that calls them belongs in inverse. Keep them separate.

Decision to make 👇

Both options give the 3rd party a standalone, light-dep forward solver and let the user own the fit. They differ only in packaging. The prerequisite refactors above are required either way.

Option A — One repo, two install profiles (extras):

  • pip install tsadar → kernel only (jax/equinox/interpax/scipy). Drops into a differentiable solver; nothing heavy loads.
  • pip install tsadar[fit] → adds optax/jaxopt/mlflow/pandas/opencv for the full pipeline.
  • ✅ Lowest overhead, single CI/version, boundaries still enforced as packages. Cross-cutting changes stay in one PR.
  • ⚠️ Forward kernel can't release on an independent cadence; one issue tracker for both audiences.

Option B — Two repos (tsadar-core forward + tsadar fitting app):

  • tsadar-core = the kernel; tsadar depends on it.
  • ✅ Hard wall, independent release cadence, clean story for forward-only consumers.
  • ⚠️ 2× CI/versioning; kernel changes that the app needs require a release + bump + coordinated PRs; harder local dev loop.

Recommendation: start with Option A (do the refactors, ship extras), and promote the kernel to its own repo (Option B) only if/when it needs an independent release cadence. A is reversible into B cheaply once the package boundaries are clean; the reverse is not.

Which do we want — A or B?


🤖 Generated with Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions