Add unit tests across core, gw, and pipe (+ two small fixes)#387
Merged
Conversation
lorenzopompili00
changed the title
There was a problem hiding this comment.
Pull request overview
This PR substantially expands the project’s automated test coverage across core, gw, and pipe, and includes two small source fixes needed for compatibility with NumPy > 2.0 and bilby prior serialization.
Changes:
- Added a broad set of unit tests covering core Result/sampler behaviors, GW likelihood/utilities/result behavior, and pipe CLI utilities/DAG orchestration.
- Fixed
np.Infusage inEarlyStoppingtonp.inffor NumPy > 2.0 compatibility. - Adjusted GW calibration prior serialization to coerce NumPy scalar instantiation parameters to native Python scalars before
repr()to preserve bilby round-tripping.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/pipe/test_pp_test.py | Adds tests for pp_test CLI helpers (parser/basename/results file discovery). |
| tests/pipe/test_pipe_utils.py | Adds round-trip and submission-key stripping tests for pipe utils. |
| tests/pipe/test_parser.py | Smoke tests for pipe argument parsers. |
| tests/pipe/test_dag_creator.py | Adds unit tests for DAG trigger-time logic and node orchestration (mocked). |
| tests/pipe/test_create_injections.py | Adds tests for time prior construction and error handling. |
| tests/gw/test_likelihood.py | Adds inner-product and GW likelihood/marginalization correctness tests. |
| tests/gw/test_gwutils.py | Adds tests for mismatch/window/prior expansion/standardization helpers. |
| tests/gw/test_gw_result.py | Adds integration-style tests for dingo.gw.result.Result behaviors including calibration sampling. |
| tests/gw/test_build_domain.py | Adds tests for domain construction and metadata-based domain building. |
| tests/gw/noise/test_asd_sampling.py | Adds tests for KDE ASD sampler output shapes and rescaling behavior. |
| tests/gw/noise/test_asd_parameterization.py | Adds tests for ASD parameterization utilities and reconstruction. |
| tests/gw/inference/test_gw_samplers.py | Adds tests for GW sampler validation and metadata post-processing (network not exercised). |
| tests/core/test_unconditional_density_estimation.py | Adds tests for unconditional density estimator training configuration and sampling. |
| tests/core/test_trainutils.py | Adds tests for training utilities: trackers, early stopping, runtime limits, and history writing. |
| tests/core/test_torchutils.py | Adds tests for torch utility helpers (optimizers/schedulers/splitting/etc.). |
| tests/core/test_samplers.py | Adds tests for sampler behavior (conditional/unconditional paths, log_prob behavior, context handling). |
| tests/core/test_result.py | Expands Result tests for evidence/statistics/merge-split and related behaviors. |
| tests/core/test_misc.py | Adds tests for version helper and recursive dict comparison. |
| tests/core/test_gnpeutils.py | Adds tests for GNPE IterationTracker update/pvalue behavior. |
| tests/core/test_density_interpolation.py | Adds tests for interpolated density helpers and multi-sample variants. |
| tests/core/test_build_model.py | Adds tests for model building/autocomplete behavior. |
| tests/core/test_backward_compatibility.py | Adds tests for minimum-version compatibility checks. |
| dingo/gw/result.py | Adds bilby prior repr() workaround for NumPy scalar instantiation values; minor formatting/comment tweaks. |
| dingo/core/utils/trainutils.py | Fixes np.Inf usage to np.inf for NumPy > 2.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* test: register heavy marker, deselect by default
* test: add apptainer/GPU detection helpers for integration tests
* test: add scaled-down heavy-test config (XPHM, fixed-param NPE, injection)
* test: add fixed-GPS time-segments generator for ASD dataset
* test: add CoreResult unit tests (SIR, evidence, split/merge, properties)
* test: add heavy pipeline orchestrator; fix delta-prior handling
Adds tests/integration/heavy/run_pipeline.py driving the 4-stage heavy
e2e pipeline (waveform dataset -> ASD at fixed GPS -> train -> dingo_pipe
local injection + importance sampling), reading sample efficiency from
the result file.
The delta-prior design (only chirp_mass/mass_ratio inferred, everything
else fixed) surfaced two core bugs that this fixes:
- gwutils.get_extrinsic_prior_dict: guard non-string (scalar/delta) values
- gw/result.sample_synthetic_phase: exclude DeltaFunction/Constraint params
from the within-prior log_prior so fixed params don't zero out all samples
Verified end-to-end: Sample efficiency = 14.34%, exit 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: regression-guard delta-prior synthetic phase; drop dead code
- Remove unused `import sys` from tests/integration/heavy/run_pipeline.py.
- Remove redundant local `from bilby.core.prior import DeltaFunction` inside
sample_synthetic_phase (~line 570); the module-level import (line 7) is used.
- Add tests/gw/test_delta_prior_within_prior.py: two fast unit tests (no GPU,
no waveform fixtures) guarding the fix that excludes DeltaFunction params
from the within_prior log_prior computation in sample_synthetic_phase.
Tests verify that a slightly-off-peak DeltaFunction param (simulating RA
trigger-time correction) does not zero out the entire within_prior mask.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: add Apptainer definition for heavy e2e container
* test: add heavy containerized end-to-end test (apptainer build + run)
Also adds pythonpath=["."] to pytest config so the namespace-package import
`from tests.integration.conftest import ...` resolves without __init__.py files.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: build heavy container without apt-get (subuid-free fakeroot)
Replace the nvidia/cuda + apt-get %post approach with a python:3.11
base image that requires no uid-switching. On hosts that also lack a
/etc/subuid range for the calling user (so the entire user-namespace
fakeroot mechanism is blocked by the kernel), redesign the test to use
a three-step sandbox workflow:
1. apptainer build --sandbox sandbox docker://python:3.11
(pulls base image; no %post / user-ns needed)
2. apptainer exec --writable sandbox pip install /opt/dingo
(pip writes files owned by the calling user — works without root)
3. apptainer build dingo-heavy.sif sandbox
(packs sandbox to SIF; no user-ns needed)
SETUPTOOLS_SCM_PRETEND_VERSION=0.9.9 is set before pip install so that
setuptools_scm does not error on the .git-less git-archive staging tree.
dingo-heavy.def is updated to document the equivalent recipe for
subuid-enabled hosts (removable with --fakeroot).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: default pipeline workdir to /tmp/dingo_run inside SIF container
/opt/heavy/_run is inside the read-only SIF filesystem; use /tmp/dingo_run
instead so run_pipeline.py works without --workdir when invoked via
'apptainer run'. /tmp is always writable (apptainer mounts tmpfs there).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: pin torch to cu128 wheel (CUDA 12.8 / driver 570 host)
Default PyPI torch 2.12.x links against CUDA 13.0 and fails with
'NVIDIA driver too old (version 12080)' on a host running driver 570
(CUDA 12.8 max). Install torch+torchvision from the cu128 index
(torch 2.11.0+cu128) before installing dingo so that the CUDA runtime
is within driver compatibility.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: time each heavy-pipeline stage and guard against slow steps
- run_pipeline.py: add timed_stage() helper, wrap each of the 4 stages
(waveform_dataset, asd_dataset, train, inference) with individual timing.
Print STAGE_SECONDS <name> = <sec> and TOTAL_SECONDS = <sec> for CI
parsing. Also rm -rf workdir on start so stale files from a previous
failed run (e.g. history.txt) don't block reuse of the same /tmp path.
- test_heavy_container.py: add _STAGE_CEILINGS, _parse_stage_times /
_parse_total_seconds / _print_timing_table helpers; print a per-stage
timing table in -s output; assert each stage is under its ceiling (loose
2.5-3x smoke guards tied to untuned config, revisit after Task 7 tuning).
- dingo/gw/result.py: fix pandas >= 3.0 incompatibility in
sample_synthetic_phase: pass dict(theta) instead of a DataFrame to
bilby's PriorDict.evaluate_constraints so dict.values() is callable.
With pandas 3.0, DataFrame.values is a property (not a method), so
bilby's try/except falls through to np.ones_like(DataFrame) which
returns a 2-D (N, 18) array, then breaks element-wise multiplication.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: calibrate per-stage ceilings from first container run
Update _STAGE_CEILINGS with observed values from the first successful
container run (A100, apptainer --nv, untuned config):
waveform_dataset 82 s -> 250 s ceiling (~3x)
asd_dataset 30 s -> 125 s ceiling (generous, download-bound)
train 324 s -> 1000 s ceiling (~3x)
inference 548 s -> 1650 s ceiling (~3x, synthetic-phase grid)
The original on-host baseline numbers (63/41/1769/120 s) were too
different from container timing to use directly as ceiling anchors.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: add GW150914 GWOSC real-data stage; tune fixed params to GW150914 ML
Add a 5th pipeline stage that downloads real O1 strain data from GWOSC
(trigger-time=GW150914) and runs end-to-end inference with the same
scaled-down network, verifying that GWOSC download + full dingo_pipe
pipeline works on a real event.
Fixed parameters updated to GW150914 GWTC-1 ML values in all configs:
theta_jn=2.68, luminosity_distance=439 Msun, dec=-1.21, ra=1.68,
zero spin. Inferred parameters (chirp_mass, mass_ratio) unchanged.
New config gw150914.ini: trigger-time=GW150914 (resolved via gwosc),
channel-dict={H1:GWOSC, L1:GWOSC}, duration=4, IS=True with PhaseRecovery.
run_pipeline.py: stage 5 runs dingo_pipe gw150914.ini and its bash script,
prints GW150914_EFFICIENCY, GW150914_CHIRP_MASS, GW150914_MASS_RATIO,
GW150914_RESULT_FILE, and STAGE_SECONDS gw150914.
test_heavy_container.py: parses GW150914 output lines, asserts efficiency
positive-finite, chirp_mass in prior [20,40] and within ±8 Msun of 31.2
(loose smoke guard), mass_ratio in [0.5,1.0]. Stage ceiling 2200s (~3x
observed 736s).
Observed on A100: GW150914 stage 736s, chirp_mass 33.4 Msun, mass_ratio
0.61, efficiency 0.02%. Test passes in ~34 min total.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: address final-review findings (comment accuracy, mass_ratio guard doc, error reporting, timeout)
- result.py: reword evaluate_constraints comment to describe bilby DataFrame-
handling bug rather than attributing it to a pandas version
- test_heavy_container.py: document mass_ratio guard as SANITY BOUND ONLY
(observed ~0.61 vs truth ~0.86; chirp_mass ±8 Msun is the real guard)
- run_pipeline.py: capture last load exception in find_is_efficiency so a
real regression surfaces the underlying error in the RuntimeError message
- test_heavy_container.py: raise subprocess timeout to
sum(_STAGE_CEILINGS) + 30 min so per-stage ceiling assertions fire before
TimeoutExpired on a pathological slow run (~87 min ceilings vs old 60 min)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: drop redundant dingo-heavy.def; sandbox build is the single path
The heavy container is built entirely in Python by test_heavy_container.py
(sandbox -> exec --writable install -> pack to SIF), which works with or
without /etc/subuid. The .def file was never referenced by any code and only
documented the fakeroot recipe its %post already replicates, so remove it and
reframe the wrapper docstring as the universal build path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* removed extra comment
* test: drop heavy-tests-only CoreResult additions from this branch
Revert tests/core/test_result.py to its hackathon/tests baseline so the
heavy-container PR adds only the integration test. The extra CoreResult unit
tests (commit 9574396) stay in history to be re-added on the test branch.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: drop delta-prior unit test from heavy-container PR
Remove tests/gw/test_delta_prior_within_prior.py so this branch adds only the
integration test. The dingo/gw/result.py delta-prior handling it guarded stays,
since the integration synthetic-phase stage depends on it; the unit test itself
(commit c2cfb1b) remains in history to be re-added on the test branch.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: flatten heavy/ into tests/integration; inline time-segments
- Move run_pipeline.py and config/ up from tests/integration/heavy/ to
tests/integration/, dropping the extra nesting (heavy/ removed).
- Inline make_time_segments.py's tiny pickle write into run_pipeline.py's
ASD stage; delete the standalone module and its subprocess hop.
- Stage the pipeline driver + config into /opt/pipeline in the container.
- Drop implementation-task references (Task 6/7, "search target") from
the timing/efficiency comments.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* added file tree structure to documentation
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nihargupte-ph
left a comment
nihargupte-ph
left a comment
Collaborator
There was a problem hiding this comment.
Overall a good PR thanks Lorenzo. The only issue I have is that most of the tests are related to typing or checking inputs and outputs are reasonable. But many of these would be trivially satisfied. I guess that's fine though, that is the point of a test. In the future, it will be better for people to write tests as they write functions, as it is hard to come up with tests after the fact.
nihargupte-ph
requested changes
Jul 10, 2026
nihargupte-ph
left a comment
nihargupte-ph
left a comment
Collaborator
There was a problem hiding this comment.
I think we should wait for the changes to the sampler befor merging. We can pass the sampler branch to claude and ask it to update the tests.
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.
Substantially expands the test suite — from 101 to 314 passing tests — focused on previously-untested logic, with cheap fixtures so it stays fast.
Source fixes (both surfaced by the new tests, under numpy>2.0):
trainutils.py:np.Inf->np.infgw/result.py::sample_calibration_parameters: cast numpy-scalar prior params to native Python beforerepr(), so the calibration path round-trips through bilby (works around an upstream bilbyPrior.__repr__bug; PR proposed upstream Make Prior.__repr__ round-trippable for numpy scalar parameters bilby-dev/bilby#1108).🤖 Generated with Claude Code