GUI + zfit: fix register_channels crash + expose picasso 0.11 module params - #5
Open
Heerpa wants to merge 21 commits into
Open
GUI + zfit: fix register_channels crash + expose picasso 0.11 module params#5Heerpa wants to merge 21 commits into
Heerpa wants to merge 21 commits into
Conversation
Adding register_channels as an abstract method on AbstractModuleCollection required every concrete subclass to implement it. AutoPicasso and ConfluenceReporter were updated, but the GUI's ModuleDescriptor was missed, so ModuleDescriptor() (called from gui.Window.__init__) raised "Can't instantiate abstract class ModuleDescriptor with abstract method register_channels" and the GUI wouldn't open. Add the register_channels GUI descriptor (parameters_spec / results_spec for bead_movies, box_size, min_gradient, model, reference, filepaths). Also add a regression guard: the GUI test `window` fixture swallows any Window() construction error as a pytest.skip, which is why CI stayed green while the GUI was broken. The new test asserts ModuleDescriptor has no leftover abstract methods and is constructible, so a future missing descriptor fails loudly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The localize module gained fitting_method / spline_calibration / camera_calibration / eps / max_it (picasso 0.11), but the GUI ModuleDescriptor.localize() descriptor still only listed box_size / fit_parallel / locs_vs_frame, so none of the new parameters were selectable in the workflow builder. Add them to the descriptor: fitting_method as a dropdown of the picasso 0.11 methods (gausslq/gaussmle/rotated/spherical/spline + -gpu variants + avg), and spline_calibration / camera_calibration as file-path fields (spline_calibration is where the spline-PSF calibration is entered), plus eps / max_it. This is the same descriptor-completeness gap as the register_channels crash, now closed for the localize parameters too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Heerpa
changed the title
Two more picasso 0.11 descriptor-completeness gaps closed: identify (GUI): expose the background-suppression / scoping options the module already accepts - temporal_median_window / _stride, gaussian_filter_sigma, roi, frame_bounds, identify_parallel. zfit: add optional fitting_method (gausslq/gaussmle, so picasso computes the axial precision for the fitter the localize step used), gpu (numba CUDA), and filter (z-fit RMSD filter, default 0 = previous behaviour), forwarded to picasso.zfit.zfit. Guard against zfit.zfit returning (None, None) instead of crashing on the z-histogram. Wire these through the AbstractModuleCollection contract and the GUI zfit descriptor. Also align the localize spline_calibration / camera_calibration GUI fields to the "path" type (matching zfit's fp_calibration) so they get a file browser. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Heerpa
changed the title
zfit: fitting_method now defaults to "auto" and is inferred from the
"Fit method" the localize module records in the info list (map any
gaussmle* variant to gaussmle, else gausslq), so it no longer has to be
set to match the localize step by hand. Still overridable; the resolved
method is recorded in the zfit info. GUI dropdown gains an "auto" default.
GUI: add conditional field visibility. A parameter spec may declare
"visible_if": {control_param: [values]} and its row is shown only while
the controlling widget holds one of those values (reusing the existing
row_widget.setVisible pattern). Applied so localize's spline_calibration
appears only for the spline fitting methods.
Also harden _load_calibration against an empty-string path (treat as
unset), so leaving an optional calibration field blank in the GUI does
not call io.load_*_calibration("").
Tests: unit test for _infer_zfit_fitting_method (which also caught a
decorator-placement slip), and a GUI test that spline_calibration
visibility follows the fitting_method dropdown.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This major update significantly improves run visibility, diagnosability,
and control for local and SLURM cluster workflows.
- **Progress tracking** for workflow runs (overall and per module/dataset) is
now recorded in `progress.json` and emitted to the log. Intra-module
progress is wired into long Picasso calls via `PicassoProgressProxy`.
- A new **GUI live progress monitor** (Run tab) provides real-time status.
It fuses SLURM job state with workflow-internal progress, showing
an overall bar and a per-module/per-dataset tree, enabling better
diagnostics for failures (e.g., "OUT_OF_MEMORY during module X localize").
- **Cooperative abort**: Runs can be gracefully stopped at the next module
boundary (or within long Picasso calls) by dropping an `abort.flag` file.
The "Cancel Job" button now requests this graceful abort before `scancel`.
- **Enhanced SLURM job submission**:
- A new **Partition** dropdown allows specifying `#SBATCH --partition`.
- A new **Modules** field allows `module load` commands (e.g., `cuda/X.Y`),
which are included in the job script, along with `export PYTHONNOUSERSITE=1`.
This is crucial for GPU fitting on module-based HPC systems.
- The `localize` module gains a **fail-fast GPU fitting guard** that checks
`numba.cuda.is_available()` upfront, providing actionable errors before
a long job aborts deep within Picasso.
Make spline-mle-gpu runnable on the cluster and fix the stuck-identify / looks-hung failure modes surfaced while debugging a 3D multicolor run. SLURM / GPU (Run tab + generator): - Add a Partition dropdown (per-cluster from config SlurmPartitions, editable) emitting #SBATCH --partition; GPU nodes need a dedicated partition or a --gres=gpu job never schedules. - Add a Modules field (per-cluster from ClusterEnvironment.<host>.Modules, editable) so the job `module load`s a CUDA toolkit (provides libNVVM, sets CUDA_HOME so numba.cuda.is_available() is True); also export PYTHONNOUSERSITE=1 so a stray ~/.local numba can't shadow the env. - config_template.yaml gains SlurmPartitions, ClusterEnvironment Modules, and SlurmDefault gpus/partition defaults. Respect picasso's own defaults for optional args (fixes stuck identify): - identify/localize no longer forward the GUI's empty/minimum sentinels. temporal_median_window: 1 was a no-op median that still switched the filter on, forcing the slow filtered read path. Now roi/frame_bounds only when non-empty, temporal median only for window >= 2, gaussian only for sigma > 0, empty fitting_method -> default, non-positive eps/max_it -> None. GUI off-sentinels lowered to 0. Fail-fast + observability: - localize and zfit reject a GPU method up front when numba.cuda.is_available() is False, with an actionable message, instead of aborting deep inside picasso. - localize logs start/end anchor lines (method, spot count, duration, spots/s) and wires cut_progress_callback so the spot-extraction phase reports progress too. Includes the live progress-monitoring helpers (progress.py read_all_progress / stage helpers, tests). Tests extended across identify/localize/zfit; full unit suite green, black + flake8 clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Report the workflow's real exit status and stop the live-progress monitor
from misreporting a dead run as COMPLETED.
- SLURM script: capture the srun step's exit code (PW_RC=$?) and end the
batch script with `exit ${PW_RC:-0}`. A bare `srun ...` followed by an
`echo` made the batch script exit 0 even when the step was cancelled
(SIGTERM 143 / MPI teardown), so SLURM reported a killed run as COMPLETED.
- Live-progress monitor: a job SLURM calls COMPLETED while tracked progress
never reached 100% is now flagged amber ("COMPLETED but workflow
unfinished (stopped at <module>)") instead of a misleading green chip.
- Live-progress box layout: only the module tree grows vertically on resize;
the job-state chip and overall progress bar keep their natural height
(fixed vertical size policy; the tree gets Expanding + the layout stretch,
and a 180px minimum replacing the old 180px maximum).
Tests: exit-code propagation in the generated script; amber flag shown when
unfinished and plain COMPLETED when done. Full unit suite green, black +
flake8 clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The monitor polls the parent results folder, which accumulates one subfolder per run, so it read every past run's progress.json. While a resubmitted job was still PENDING (its own progress not written yet), the monitor showed the previous run's completed progress. Scope the polled states to the current SLURM job id (the run token woven into every report_name via metaworkflow._run_token), matched as a whole _<id> token. A pending resubmission now shows PENDING with an empty tree rather than a prior run's stages; local monitoring (no job id) is unchanged. Tests: job-id filtering, pending->empty, no-job-id passthrough, whole-token matching, and an end-to-end PENDING display. Full unit suite green, black + flake8 clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SLURM opens the --output/--error files at job launch and does not create their parent directory, so a missing logs/ dir silently dropped the job's stdout/stderr -- exactly where a traceback or OOM message would land. Create the logs/ directory (on the cluster-shared results filesystem) before submitting, and name the files with %j (the job id) rather than %A (the array-master id, which only coincides with %j by accident for non-array jobs). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The recurring identify/localize stalls were the movie's per-frame reads crawling/hanging over the shared (NFS) filesystem, not OOM or a logic bug: runs froze mid-identify with ~1.3 GB RSS on separate nodes, both at once (same NFS server), and were cancelled -> SIGTERM. Fix the root cause by reading the movie from local disk. load_dataset_movie gains an optional stage_to_local flag (GUI checkbox, off by default): it copies the movie to node-local scratch ($SLURM_TMPDIR / $TMPDIR, auto-cleaned by SLURM) before loading, so every subsequent frame read is local. A split OME-TIFF/MMStack series (<base>_<N>.ome.tif) and a .raw sidecar are staged as a self-contained set, without pulling in an unrelated neighbouring movie. Best-effort: any failure logs a warning and falls back to the original network path, so staging never aborts a run. Tests: series discovery (incl. not over-matching Pos3 vs Pos30), a staging round-trip, and the error fallback. Full unit suite green, black + flake8 clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The monitor rebuilds the tree from scratch on every 15s poll (clear + repopulate), which collapsed everything and reset to the "expand if running" default -- undoing the expand/collapse of anyone investigating a stage. Capture the current expansion before the rebuild, keyed by stable ids on the collapsible items (the aggregation root and the per-dataset stage groups, keyed by dataset slot so state carries over when a pending dataset becomes a live stage), and reapply it afterwards. Items new to a refresh keep the builder default. Tests: expand and collapse both persist across a refresh. GUI suite green, black + flake8 clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_plot_locs_vs_frame hardcoded the Gaussian PSF width columns sx/sy, so a spline localize crashed with KeyError: 'sx' (spline locs carry z/lpz, not sx/sy). Adapt the second panel to the columns the fitter actually produced: sx/sy when present, else the axial position z, else a single photons panel. Test covers both the Gaussian (sx/sy) and spline (z, no widths) column sets. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When <=2 gold picks are found, find_gold built the empty gold set as
pd.DataFrame(columns=self.locs.columns), whose columns default to object
dtype. io.save_locs -> DataFrame.to_records -> h5py then failed with
"Object dtype dtype('O') has no native HDF5 equivalent", aborting the run
on any dataset with no gold. Slice self.locs to zero rows instead, which
keeps each column's numeric dtype so the empty file saves.
Regression test drives the empty branch through the real io.save_locs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
identify recorded its forwarded options in self.info via new_info.update(identify_kwargs), but identify_kwargs now also carries the live progress_callback (PicassoProgressProxy) and abort_callback wired in for the progress monitor. Those hold a threading.Lock (via ProgressManager), so a later io.save_locs -> save_info -> yaml.dump failed with "TypeError: cannot pickle '_thread.lock' object" (surfacing at the find_gold save). Record only the real identify parameters (filters/ROI/frame bounds), not the callbacks. Affects any progress-tracked run that saves locs. Regression test asserts the callbacks reach picasso but not self.info, and that the info list is yaml-serializable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Progress completeness fixes for aggregation runs, plus one more spline column-safety fix from the module audit. - workflow.py: move parameter_command_executor.run (the $-command resolution, e.g. $get_prior_result) inside the per-module try/except. A failure there -- such as referencing a module absent from this workflow -- previously escaped before module_start/module_end/finish, leaving the single's progress.json stuck at RUNNING; the live monitor then showed the dataset as still running long after the rank had failed it and moved on. Now it is recorded as a module failure and the run's progress is finalized. - gui.py: merge the per-rank aggregation progress states. Each SLURM rank writes its own aggregation file advancing only the datasets it owns (round-robin); the monitor now combines them (most-advanced state per dataset) instead of showing a single rank's partial view. - analyse.py find_similar: save the empty pick set with the localizations' numeric dtypes instead of a hardcoded Gaussian column list (sx/sy/ellipticity, object dtype), same HDF5 crash as find_gold. Tests for the multi-rank merge; find_similar/find_gold covered. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
g5m's `calibration` is an astigmatism z-calibration, only used to derive lpz when the locs lack it. Spline fits provide lpz directly, so no calibration is needed -- but the module always ran g5m in its default astigmatism mode and did not expose the mode. Add a `mode` option (auto/astigmatism/spline, GUI-selectable) forwarded to g5m. `auto` (default) infers the z model from the recorded fit method (_infer_gmm_mode): spline* -> "spline" (reads lpz from the locs), else "astigmatism". Gaussian workflows are unchanged; spline workflows no longer need an (astigmatism) calibration. Pixel size is read from the metadata by g5m, so it is not forwarded separately. Test covers the fit-method inference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A LAPACK SIGSEGV inside a picasso fit/CRLB call (e.g. spline-mle CRLB via np.linalg.pinv on a non-finite Fisher matrix) leaves no Python traceback -- the process just dies and SLURM cancels the whole step, taking down the other MPI rank as collateral. Enabling faulthandler makes the next such crash dump the Python->C frames (inherited by multiprocessing workers), pinpointing the exact precision.py pinv site so the right guard can be added upstream. Negligible overhead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two checkboxes on the Run tab, wired through assemble_slurm_commands: - faulthandler (on by default): export PYTHONFAULTHANDLER=1 so a native crash (LAPACK/CUDA fault in a fit/CRLB call) prints a Python->C traceback instead of a silent segfault. Replaces the previously hardcoded export. - debug: CPU-only (off): export CUDA_VISIBLE_DEVICES="" so cuda.is_available() is False and the fit/CRLB run on the fully-guarded CPU kernels. picasso runs the spline CRLB on the GPU whenever one is visible -- even for the CPU 'spline-mle' fit method -- so this is the lever that bisects a GPU-side crash (which dies as a hard SIGSEGV, not a catchable CudaAPIError) from the CPU path. Tests cover both flags at the command level and the widget defaults. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Static i % size assignment made the aggregation makespan the max per-rank load, so one rank drawing the spot-heavy datasets set the wall-clock while others idled. Replace it with greedy dynamic self-scheduling: each rank walks the dataset list and races to claim the next one via an atomic mkdir on the shared result folder, running the winner and only then advancing -- so a free rank always grabs the next unclaimed dataset. - Claim dir is scoped by SLURM_JOB_ID (_pwf_claims/<job>/), separating "in progress this launch" (claim) from "finished" (persistent per-folder completion marker), so a stale claim from a crashed attempt never blocks a rerun. - os.mkdir is atomic on NFS (server serialises MKDIR) -> exactly one owner per dataset. An unexpected FS error runs the dataset locally (redundant but correct) rather than dropping it and hanging rank 0's barrier. - Coordination stays on the shared filesystem (no MPI). The completion-marker barrier and rank-0 disk gather were already assignment-agnostic, so only the one assignment line changed. Single-task runs skip claiming. Tests: exclusivity + full coverage across two ranks, job-scoped claim dir with local fallback, FS-error-runs-locally. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… requirements and installation instructions
…r accurate monitoring
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.
Summary
Completes the picasso 0.11 parameter surface across the GUI
ModuleDescriptorand thezfitmodule. Started as a GUI-crash hotfix, now also exposes the new 0.11 parameters that were added to the modules but never wired into the builder.1. Crash fix
register_channelswas added as an abstractAbstractModuleCollectionmethod but its GUI descriptor was missing, soModuleDescriptor()(fromgui.Window.__init__) raisedTypeError: Can't instantiate abstract class ModuleDescriptor with abstract method register_channels— the GUI would not open. Added the descriptor.2. GUI: expose new module params
fitting_method(dropdown of the picasso methods incl. spline + -gpu variants + avg),spline_calibrationandcamera_calibration(file-path fields —spline_calibrationis where the spline-PSF calibration is entered),eps,max_it.temporal_median_window/_stride,gaussian_filter_sigma,roi,frame_bounds,identify_parallel.3. zfit module + GUI
fitting_method(gausslq/gaussmle— match the localize step so picasso 0.11 computes axial precision correctly),gpu(numba CUDA), andfilter(z-fit RMSD filter; default 0 preserves current behaviour), forwarded topicasso.zfit.zfit. Guards againstzfit.zfitreturning(None, None). Wired through the contract and the GUI descriptor.Regression guard
The GUI test
windowfixture swallows anyWindow()construction error as apytest.skip, so the abstract-instantiation crash made every GUI test skip rather than fail — that is why CI stayed green while the GUI was broken. Added a test assertingModuleDescriptor.__abstractmethods__is empty and it is constructible, so a future missing descriptor fails loudly.All defaults preserve current behaviour. Full unit suite green (0 failures); lint clean.
🤖 Generated with Claude Code