Repository navigation
Port srf2stoch to Python - #128
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
There was a problem hiding this comment.
Pull request overview
This PR ports stoch generation from the external srf2stoch binary to a pure-Python implementation, integrating SRF→Stoch conversion directly into the generate-stoch CLI and simplifying container tooling.
Changes:
- Implement SRF→Stoch conversion in Python using NumPy/SciPy (box-averaging slip, rupture time, and slip-weighted rise).
- Remove the
srf2stochbinary dependency from both the CLI and the container build. - Bump
source_modellingminimum/version pin to2026.07.3/2026.7.3.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| workflow/scripts/generate_stoch.py | Replaces external srf2stoch invocation with in-process Python conversion and updates CLI/docs accordingly. |
| container/runner.def | Stops building srf2stoch in the runner image. |
| pyproject.toml | Updates source_modelling minimum version requirement. |
| uv.lock | Updates lockfile entries (incl. source-modelling 2026.7.3 and related metadata). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
15bce20 to
fcbdd14
Compare
52a9ee3 to
b4d0555
Compare
b4d0555 to
fcf9906
Compare
fcf9906 to
e6a0593
Compare
e6a0593 to
a092176
Compare
a092176 to
a799327
Compare
a799327 to
d015a37
Compare
d015a37 to
44cd2d3
Compare
eee3b7f to
eb147f0
Compare
65eb54c to
f943533
Compare
c1cad68 to
3f3692c
Compare
514b986 to
c7e9844
Compare
c7e9844 to
4991337
Compare
b43435d to
5579207
Compare
5579207 to
8178d62
Compare
8178d62 to
5de237d
Compare
lispandfound
left a comment
lispandfound
left a comment
There was a problem hiding this comment.
Code review by Claude Code (run locally, since the review action is currently broken). 8 finding(s).
| ] | ||
| ) | ||
| srf_file = srf.read_srf(srf_ffp) | ||
| dx = stoch_config.stoch_dx |
There was a problem hiding this comment.
Two special cases were removed: point sources used the SRF grid (len/nstk, wid/ndip), and other planes were capped at min(stoch_dx, min_length/2). Every plane now uses the 2 km stoch_dx/stoch_dy, even when the plane is smaller than one cell.
Failure scenario: A point-source SRF, or a 0.5 km segment in a multi-fault rupture, becomes a single 2x2 km stoch cell. Moment is conserved, but HF reconstructs a plane up to 4x larger than the real source. Combined with the dip padding, the source centre also sits about dy/2 * sin(dip) deeper than it should. If this is intended, it needs a comment. If not, keep a per-plane dx for sub-cell planes.
There was a problem hiding this comment.
The stoch dx behaviour is expected. The old hack to adjust the stoch dx was not desired it was an artifact of the limitations in srf2stoch. As for the point source case: yes I guess this could be an issue but I don't think it will matter, you can just adjust the stoch dx/dy if you want that.
There was a problem hiding this comment.
Cell registration is a bug, which I have addressed.
Replaces the `srf2stoch` binary with an in-process conversion, and drops it from the container build. The conversion is a box average expressed as a sparse fractional-overlap matrix: row j of the kernel holds the overlap weights between coarse bin j and the fine cells it spans. The special case where the two grids span exactly the same length is what srf2stoch.c implements; the sparse form generalises it without materialising the empty overlaps, and handles the padded case where they do not. That padded case is why this was worth porting. The HF code demands a single dx/dy across every SRF segment, and `nx = ceil(len / dx)` means the coarse grid is generally *longer* than the plane it covers. The overhang is split evenly between the two ends, so the grids stay concentric -- the stoch format records only a centre point and an `nx * dx` extent, so off-centre padding would sit the slip distribution in the wrong place on the plane HF reconstructs. Edge bins then carry weights summing to the covered fraction rather than to 1, which is what conserves total slip*area rather than cell value. Quantities that are not spread over a cell are handled separately. Rupture time is a time, so it is divided by the covered fraction; every cell is partially covered because nx and ny round up, so this never divides by zero. Rise time is slip-averaged, and there the coverage factor cancels, so it must *not* be divided out again. Rake is averaged as a circular mean weighted by slip -- the arithmetic mean of -179 and 179 degrees is 0, which is the opposite of the right answer. A plane with no slip anywhere has no slip-weighted mean, so it falls back to an unweighted average. This changes every stoch file, and therefore every high-frequency result. The previous binary's outputs were not moment-conserving for segments whose length was not an exact multiple of the stoch dx. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
AndrewRidden-Harper
left a comment
AndrewRidden-Harper
left a comment
There was a problem hiding this comment.
very nice docstrings and comments
This PR removes the
srf2stochbinary from the workflow and replaces it with a pure python implementation that does the same thing with some important improvements:dxin the SRF and the provided stoch dx didn't place nice. The old wrapper worked around this with the min_length/2 clamp and a special case for point sources. In the Python port, these arithmetic issues are avoided.