Release 0.4.1: NoisePy co-install + last obspy call off the CCF path - #7
Merged
Conversation
Picks up the working-tree work that landed after v0.4.0 and hardens one part of it. The changes as found: gps2dist_azimuth_np ports obspy's Vincenty inverse, which was the last obspy call on the cross-correlation path since noisepy's cc_parameters writes dist/azi/baz into every saved CCF; preprocess_raw_np rounds segment start times to whole microseconds, as obspy's miniSEED reader does, because noisepy derives its sub-sample correction from UTCDateTime.microsecond and exact nanoseconds shifted the output ~1e-8 relative; pretrimmed=False skips the alignment guard for callers passing whole segments, which is what obspy.read hands noisepy; and the boto3 floor drops to >=1.26 because noisepy-seis-io pins s3fs==2023.4.0 -> botocore<1.29.162, which >=1.28 excludes. Added here: the geodetics tests asserted exact equality against whichever branch obspy took, so they failed outright on any machine with geographiclib installed — obspy delegates to it when present. Verified by installing it: 2 failures, distances differing ~5e-6 m over 566 km. The exact tests now skip under HAS_GEOGRAPHICLIB and a tolerance test runs either way so coverage is not lost. That test needed the azimuth compared modulo 360, since Vincenty reports a due-south back-azimuth as 360.0 where geographiclib says 0.0 — the port is right and the first cut of my test was wrong. Divergence documented on the port itself: bit-identity holds against a default obspy install, not against every one. 274 unit tests and 57 precision tests pass, verified both with and without geographiclib. Not tagged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR prepares the 0.4.1 release focused on making NoisePy’s “obspy-free” cross-correlation path truly usable by (1) removing the last ObsPy geodetics call from the CCF metadata path and (2) ensuring the preprocessing chain remains bit-identical to the default ObsPy behavior while allowing co-installation with NoisePy’s pinned S3 dependency stack.
Changes:
- Added an ObsPy Vincenty-inverse port (
gps2dist_azimuth_np) and precision tests that handle ObsPy’s geographiclib-vs-Vincenty branching. - Updated
preprocess_raw_npto (a) optionally skip the sample-grid alignment guard for whole-segment inputs and (b) round segment start times to whole microseconds to match ObsPy miniSEED behavior. - Relaxed the
boto3lower bound and updated release/docs/changelog to reflect 0.4.1.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/precision/test_preprocess_equivalence.py | Adds precision tests for gps2dist_azimuth_np, including branch-aware behavior when geographiclib is installed. |
| seisfetch/contrib/obspy_ports.py | Adds gps2dist_azimuth_np (Vincenty inverse) to remove the last ObsPy call from the CCF metadata path. |
| seisfetch/contrib/noisepy_adapter.py | Adds pretrimmed flag to the alignment guard and rounds t0_ns to microseconds for bit-identity with ObsPy. |
| pyproject.toml | Bumps version to 0.4.1 and relaxes boto3 lower bound to allow NoisePy co-installation. |
| docs/noisepy-obspy-replacement-report.md | Updates report status and documents the remaining gaps closed in this release. |
| CHANGELOG.md | Adds the 0.4.1 release entry describing the added port, preprocessing fixes, and dependency change. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # to carry the same start time obspy would: an 80 ns difference shifts | ||
| # nfric by ~1.6e-6 and the interpolated output by ~1e-8 relative, which | ||
| # is small but NOT bit-identical, and bit-identity is the contract. | ||
| t0_ns = int(round(t0_ns / 1000.0)) * 1000 |
Comment on lines
+241
to
+242
| if math.isclose(lat1, lat2) and math.isclose(lon1, lon2): | ||
| return 0.0, 0.0, 0.0 |
| lon2: float, | ||
| a: float = WGS84_A, | ||
| f: float = WGS84_F, | ||
| ): |
Accepted two, rejected one. The microsecond rounding used int(round(t0_ns / 1000.0)) * 1000. At epoch nanosecond magnitudes (~1.6e18) the float quotient has a 0.25 ulp, so the sub-microsecond part is quantized to 250 ns steps; measured against pure integer rounding it picks a different microsecond for 13.2% of arbitrary inputs (26488 of 200000), and round() is half-to-even on exact .5. Now ((t0_ns + 500) // 1000) * 1000. Investigating it also showed the changelog overstated the motivation: no sub-microsecond start exists in 3026 segments across every cached SCEDC, NCEDC and EarthScope day file, and both roundings match obspy on every fixture. So this is a guard, not an observed repair, and the entry now says so instead of describing a break I cannot reproduce. Rejected the suggestion to replace math.isclose with exact equality in the identical-point fast path. obspy's own calc_vincenty_inverse uses math.isclose there (geodetics/base.py line 60), so exact equality would introduce the divergence the comment warns about rather than remove it. Verified: a pair 1e-9 degrees apart returns (0.0, 0.0, 0.0) from both. Also added the missing return annotation on gps2dist_azimuth_np. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Follows 0.4.0. The theme is making NoisePy's obspy-free data path actually usable: seisfetch has to install alongside noisepy's pinned S3 stack, and the cross-correlation path has to stop calling obspy entirely.
Added
contrib.obspy_ports.gps2dist_azimuth_np— a port of obspy's Vincenty inverse. noisepy'scc_parameterswrites dist/azi/baz into every saved cross-correlation, so this was the last obspy call left on the CCF path. Reproduced statement for statement, including the_normalize_longitudesubtraction loop (a modulo differs in the last ulp and that propagates into the returned distance) and the degenerate-case branches.Fixed
preprocess_raw_nprounds segment start times to whole microseconds, as obspy's miniSEED reader does. pymseed keeps exact nanoseconds; noisepy derives its sub-sample correction fromUTCDateTime.microsecond. On a day file starting at 00:00:00.019537920 the two chains computednfric1.6e-6 apart and output differed ~1e-8 relative — small, but bit-identity is the contract.preprocess_raw_np(..., pretrimmed=False)skips the sample-grid alignment guard for callers passing whole segments, which is exactly whatobspy.readhands noisepy. The guard still applies to pre-trimmed input, where it is load-bearing.geographiclibwhen installed and only falls back to its own Vincenty otherwise, so the tests failed outright on any machine with it present — verified by installing it: 2 failures, ~5e-6 m over 566 km. Exact tests now skip underHAS_GEOGRAPHICLIB; a tolerance test runs either way so coverage is not silently lost. That test compares azimuth modulo 360, since Vincenty reports a due-south back-azimuth as360.0where geographiclib says0.0.Changed
boto3floor relaxed to>=1.26(was>=1.28). Without this NoisePy cannot install seisfetch at all:noisepy-seis-iopinss3fs==2023.4.0, forcingaiobotocore 2.5.2andbotocore<1.29.162, a rangeboto3>=1.28excludes.s3.pyuses only client/Session/paginators,botocore.UNSIGNEDandClientError. The pixi.lock churn is large but required — the lock encodes the requirement string; resolved versions are unchanged.Scope note
Bit-identity on the CCF metadata path now holds against a default obspy install rather than every obspy install. The geographiclib divergence is ~5 microns over 500 km, far below what CCF metadata resolves, but the project's bar is bit-identity so it is stated rather than assumed — documented on the port and in the changelog.
Verification
274 unit tests, 57 precision tests, verified both with and without geographiclib installed. Lint and format clean.
🤖 Generated with Claude Code