Repository navigation
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
==========================================
- Coverage 90.81% 89.09% -1.73%
==========================================
Files 21 23 +2
Lines 1470 2649 +1179
Branches 305 538 +233
==========================================
+ Hits 1335 2360 +1025
- Misses 76 177 +101
- Partials 59 112 +53 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
CIFTI fast path - Fix pconn rows stored transposed: NIfTI stores the CIFTI matrix column-major, so the raw read now reorders it to the row-major layout nibabel returns. - Hash every non-scalar MatrixIndicesMap (both pconn parcel maps, or the shared "0,1" map), so permuted row or column parcels fall back to nibabel and are rejected. - Build the plan from the reference file's own on-disk header and XML, through the same checks every follower gets, instead of nibabel's in-memory header (which hides byte order and scaling). Big-endian cohorts now use the fast path; scaled data always goes to nibabel. - Check structure from header bytes only: no nibabel axis is rebuilt and the XML is digested in place, so per-file cost stays below the PR's original. - Hash public nibabel axis properties in cifti_spatial_signature. CIFTI export - h5_to_cifti writes float32 results instead of re-quantizing them into the template's on-disk dtype (e.g. int16 with scl_slope). TileDB publication - Converting into an existing TileDB root merges into it again, as on main: arrays written by the run replace their namesakes, other scalars are kept, and an interrupted publication is rolled back. CLI, storage and loaders - Validate --workers / --s3-workers as positive integers in argparse. - Emit the --s3-workers deprecation once (FutureWarning) with one wording. - Default cifti_to_h5 / nifti_to_h5 / mif_to_h5 compression to None, like to_modelarray(). - Resolve HDF5 and TileDB compression through the shared resolver. - Write HDF5 lzf without a fallback warning; fall back from direct-chunk gzip writes only on zlib/h5py errors. - Build NIfTI/MIF source lists with the shared cohort row iterator. - Remove always-zero or duplicated telemetry keys, production branches that existed only for test doubles, and benchmark figures from help text, comments and the README.
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.
Summary
Rewrites the
modelarrayio to-modelarrayconversion path for speed without changing the output schema or breaking ModelArray reads. On real RBC freesurferpost data on CUBIC, one full 8,693-subject CIFTI scalar drops from about 3.3 h to about 7 min at one worker (27.5×), and to 69–91 s at eight workers. All 70 production scalars now convert in ≈22 min of job-array wall-clock, where the original code took 112 h for its first scalar and stalled at 15/70. Every output is bitwise-equal to the deployed reference.The design, measurements and rejected alternatives are in
design/TO_MODELARRAY_RUNTIME_OPTIMIZATION_PLAN.md.What was slow
What changed
Loader (
utils/loading.py,utils/{cifti,nifti,mif}.py,utils/s3_utils.py)ThreadPoolExecutor.fast_path_rows.MODELARRAYIO_S3_ANON=1for public buckets. Broken git-annex symlinks report an actionable error instead of being read.Storage (
storage/{h5_storage,tiledb_storage,utils}.py)MODELARRAYIO_HDF5_COMPRESSION_WORKERS, default all CPUs).--compression nonedisables the filter pipeline.CLI (
cli/runtime.py,cli/to_modelarray.py,cli/{cifti,nifti,mif}_to_h5.py,cli/parser_utils.py)--workers Nnow means one thing everywhere: the maximum number of input sources loaded concurrently, for local and S3 paths, every modality and both backends. This changes the meaning of an existing option: it previously controlled concurrent TileDB scalar writes. The resolved meaning is logged on the first run with--workers > 1.--s3-workersis a deprecated alias for--workersfor one minor release. Matching values are accepted, conflicting values rejected.--dtype float32stays the default;--dtype float64is the documented opt-in that keeps source precision end to end.--timing-jsonl PATHappends structured per-scalar and total runtime records (load/write phases, peak RSS, fragment counts, fast-path rows, TileDB pool settings).s3://URIs.tqdmdependency removed.Docs
autosectionlabelprefixing fixes duplicate-label warnings; thesphinx-argparsepatch tolerates versions withoutArgParseDomain.Benchmarks (
benchmarks/)New self-contained suite under
benchmarks/to_modelarray/: a harness that runs the full CLI across NIfTI/CIFTI/MIF × HDF5/TileDB, cohort generators (public ABIDE froms3://fcp-indi, the MIF walkthrough demo, a reproducible CIFTI synthesizer), an old-commit launcher pinned to9ab13dd, output-equality and R read gates, a comparison-report generator, and the CUBIC SLURM ladder (container env, sbatch templates, scalar-sharded job array). Pinned results live inbaselines/, the rendered CUBIC report inreports/.Cross-modality, 100 subjects, original
9ab13ddvs this branch, one worker, CUBIC (baselines/cubic_cross_modality_9ab13dd_vs_optimized_20260922.csv):Production scale, one full CIFTI scalar (
area, 8,693 subjects), CUBIC (baselines/cubic_production_scale_9ab13dd_vs_optimized_20260923.csv):Peak memory is flat across workers because the staging matrix dominates. Other modalities were not swept at production scale, so the CLI keeps the conservative default of one worker.
Deferred
--only-scalar: per-scalar cohort CSVs select the scalar for the sharded HPC recipe instead.Tests
New:
test_runtime.py,test_runtime_integration_matrix.py,test_loading.py,test_storage_utils.py,test_s3_utils.py,test_benchmark_harness.py,test_comparison_report.py,test_cubic_tools.py,test_make_cifti_cohort.py. Extended: CIFTI/NIfTI utils, HDF5/TileDB storage, CLI utils and parser tests.