Add unit testing#18
Conversation
|
Note that all changes to files in |
and mocked coverage for inspiral and merger-ringdown generators - Add unit tests for inspiral and MR mode generation utilities using mocked lalsimulation / delegated generators. - Add deterministic unit tests for waveform utilities, blending helpers, and legacy fitting functions without using mocks. utils.py * verify f22/x roundtrip consistency across x and mass ranges * verify f22 scales as M^-1 * verify Schwarzschild ISCO frequency for zero spins * verify prograde spins increase ISCO frequency * verify retrograde spins decrease ISCO frequency * verify monotonic inputs produce no detected peaks * verify sinusoidal inputs return expected peak locations * verify equatorial-plane polarization reconstruction for (2,2) mode * verify face-on symmetry gives hx = 0 * verify polarization linearity under mode scaling blend.py * verify first-value lookup on constant arrays * verify first/last index lookup on increasing arrays * verify first/last symmetry properties * verify amplitude extraction for known complex inputs * verify amplitude for real-valued inputs * verify phase unwrapping for linear complex phase * verify constant-phase inputs produce flat phase * verify frequency recovery from linear phase evolution * verify decreasing phase produces negative frequencies * verify mismatch is zero for identical waveforms * verify mismatch is one for orthogonal waveforms * verify mismatch remains within [0, 1] * verify blended series matches endpoints correctly * verify blended series smoothness across transition region * verify blended series output length * verify phase alignment reduces mismatch * verify inverse alignments cancel net phase shift legacy.py * verify zero polynomial coefficients return unity * verify quadratic polynomial evaluation for known coefficients * verify equal numerator/denominator ratio polynomial returns unity * verify fitting functions remain positive for η ∈ (0, 0.25] * verify fitting functions are finite at η = 0.01 and η = 0.25 Included test coverage: * TestGetInspiralEsigmaModesWithMock (7 tests) - verify returned dict contains requested (l,m) key - verify include_conjugate_modes=True/False controls (l,-m) - verify default return type is pycbc.TimeSeries - verify return_pycbc_timeseries=False returns numpy arrays - verify distance conversion from Mpc to SI before C call - verify mode generation called once per unique mode * TestEccentricityAtReferenceFrequencyWithMock (2 tests) - verify nearest-index lookup logic for x_ref - verify returned eccentricity matches argmin|x - x_ref| * TestGenerateLalsimModesWithMock (5 tests) - verify missing approximant propagates AttributeError -> IOError - verify linked-list traversal filters requested modes only - verify all requested modes are returned - verify unrequested modes are ignored - verify extracted mode data is returned as numpy arrays * TestGetMrModesWithMock (4 tests) - verify f_ref=None defaults to f_lower - verify coa_phase=None defaults to 0.0 - verify NRSur7dq4 routes to _generate_lalsim_modes - verify SEOBNRv5HM routes to _generate_pyseobnr_modes
Akash-Maurya-0899
left a comment
There was a problem hiding this comment.
Hi @prayush, I have added some comments. The major one is regarding the black formatting test, which may simply fail due to different black versions used by the tests vs by a developer on their local machine. The fix would be to pin a particular version black for the tests, and also add it as an optional "developer" dependency of esigmapy. More details in the comments.
| [ | ||
| sys.executable, | ||
| "-m", | ||
| "black", |
There was a problem hiding this comment.
Pin a particular version of black. The black formatting tests failed for me as I had a different version of black installed on my local environment.
It would then also be good to add this same version of black as an optional dependency for the developers only, by adding these dependencies under a dev header in the setup.py file
extras_require={
"surrogate": [
"tpi-splines",
"numba",
],
"pyseobnr": ["pyseobnr"],
"dev": [
"black==xx.xx.x", # pinned to avoid formatting disagreements
"pytest",
],
"all": [
"tpi-splines",
"numba",
"pyseobnr",
],
},
| # 1.5 lies between arr[1]=1 and arr[2]=2; nearest is equidistant → either valid | ||
| arr = np.array([0.0, 1.0, 2.0, 3.0]) | ||
| idx = find_first_value_location_in_series(arr, 1.5) | ||
| assert idx in (1, 2) |
There was a problem hiding this comment.
Shouldn't it check if the output is 1, and not 1 or 2 (because that's what find_first_value_location_in_series would output I think)? Or is it that due to floating point rounding error, it can be either 1 or 2?
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| pip install lalsuite pycbc pytest black |
There was a problem hiding this comment.
|
|
||
| class TestFindFirstValue: | ||
| def test_below_min_raises(self): | ||
| with pytest.raises(Exception, match="[Ll]ower"): |
There was a problem hiding this comment.
Not sure if this matching of "Lower/lower" in the exception message is needed; the TestFindLastValue set of tests below don't use it. If keeping it, please add a similar string matching requirement for the TestFindLastValue sets of tests below also.
| arr = np.linspace(1.0, 5.0, 50) | ||
| idx = find_last_value_location_in_series(arr, 3.0) | ||
| assert 0 <= idx < len(arr) | ||
|
|
There was a problem hiding this comment.
Counterparts of test_nearer_left_element_chosen, test_nearer_right_element_chosen, test_returns_first_bracket_index, test_exact_match_at_start of TestFindFirstValue suite of tests missing for TestFindLastValue suite of tests here.
| idx = find_first_value_location_in_series(arr, 0.7) | ||
| # First bracket is [0→1]; nearest of 0.0 and 1.0 to 0.7 is 1.0 → idx 1 | ||
| assert idx <= 1 | ||
|
|
There was a problem hiding this comment.
test_result_within_valid_range like test below is missing here.
| # |3 + 4j| == 5 | ||
| wave = np.full(10, 3.0 + 4.0j) | ||
| amp = compute_amplitude(wave) | ||
| assert np.allclose(amp, 5.0) |
There was a problem hiding this comment.
Perhaps decrease the tolerances rtol and atol in np.allclose everywhere to make the tests more stringent.
| idx = self._all_idx(len(w1)) | ||
| assert mismatch_discrete(w1, w2, idx, idx) >= 0 | ||
|
|
||
| def test_subsample_indices(self): |
There was a problem hiding this comment.
Not sure why this particular test is needed...
|
|
||
| def test_case_sensitive(self): | ||
| with pytest.raises(IOError): | ||
| check_available_mr_approximants("nrsur7dq4") |
There was a problem hiding this comment.
Not a review of this PR, but looking at this, we should perhaps include support for case-insensitive MR approximant names in future.
|
|
||
| def test_lalsim_contains_seobnrv4phm(self): | ||
| assert "SEOBNRv4PHM" in LALSIM_APPROXIMANTS | ||
|
|
There was a problem hiding this comment.
Add similar checks for the supported pyseobnr approximants as well.
This PR adds substantial new unit test coverage for waveform generation, waveform utilities, blending helpers, and legacy fitting functions.
Generator and mode-generation tests
New mocked tests were added for inspiral and merger-ringdown mode generation using patched
lalsimulationand delegated generators. These tests verify:(l,m)modes,pycbc.TimeSeriesvs NumPy arrays),f_refandcoa_phase.Deterministic numerical tests
Additional pure-Python tests (without mocks) were added for
utils.py,blend.py, andlegacy.py.These tests verify:
Overall, this PR improves regression protection and numerical validation across the waveform generation and hybridization codebase while keeping the tests deterministic and lightweight for CI.