[claudio525]

Fixes #156, #157, #158, #171. Also fixes the estimator parts of #177. Supersedes #178.

Fixes

All estimators now return km, as the docstrings specify. I checked each estimator against the paper and against OpenQuake.

Function	Problem	Fix	Reference
chiou_young_14_calc_z1p0 (Japan)	Denominator used 4th powers. Z1.0 was too large by ~15 orders of magnitude.	Use squares.	CY14 Eq. 2
mod_chiou_young_14_calc_z1p0 (ASK14)	Global slope was -7.15 (CY14 value). Japan branch had the 4th power error.	Slope -7.67. Japan uses squares and 412.	ASK14 Eqs. 18, 19
campbell_bozorgina_14_calc_z2p5	California and Japan coefficients were in the wrong branches.	Swap the branches.	CB14 Eqs. 33, 34
kuehn_20_calc_z	Returned ln(Z) in metres.	exp(...) / 1000	Kuehn et al. (2020) Eq. 4.11
abrahamson_gulerce_20_calc_z2p5	Returned metres.	/ 1000	AG (2022) Eq. 9
parker_20_calc_z2p5	Returned metres.	/ 1000	Parker et al. (2022) Eqs. 12, 13

Removed regions

The coefficients for two regions do not agree with the papers. I removed these regions:

• AG20 Japan: The paper gives a slope of 0.680 (Eq. 10). The code and OpenQuake use 2.066. The source of 2.066 is not known.
• P20 Cascadia: The paper gives θ0 = 3.75, θ1 = -0.74, vμ = 500, vσ = 0.42 (Table 2). The code and OpenQuake use 3.94, -0.42, 200, 0.2. The source of these values is not known.

Tests

Adds tests/test_estimations.py. The tests come from #178, with these changes:

• A new test compares each estimator and region with the OpenQuake function.
• I removed test_mod_chiou_young_14_z1p0_shallower_than_original. This test is only true with the incorrect -7.15 slope.

Note

This change modifies the output of Z_CALC_MODEL_REGION_MAPPING. For K20, AG20 and P20, the values change by orders of magnitude. No code in oq_wrapper calls these estimators directly.

[claudio525]

@claude review

[claude]

Claude finished @claudio525's task in 3m 10s —— View job

---

Review complete ✅

• Gather context (diff, CLAUDE.md/REVIEW.md, prior review comments — none found)
• Review oq_wrapper/estimations.py changes for correctness
• Review tests/test_estimations.py
• Post findings

Findings: 0 blocking, 0 minor — no correctness bugs found in the diff.

Analysis

I checked each fix in oq_wrapper/estimations.py for internal consistency and dimensional correctness:

• kuehn_20_calc_z — previously returned the raw ln_z_ref (a log value) where a depth in km was documented/expected. np.exp(ln_z_ref) / 1000 is the right conversion; sanity-checked each region's constant (c_z_1) and the resulting depths are all physically plausible (0.5–4 km).
• chiou_young_14_calc_z1p0 (Japan) — fixing the denominator from 4th to 2nd powers also fixes the reference-rock anchor: at vs30 == 1360 numerator now equals denominator (ratio = 1 → ln(1) = 0), matching the model's intended boundary condition and the new test_chiou_young_14_z1p0_global_reference_rock test. The old code didn't satisfy this anchor, confirming it was broken.
• campbell_bozorgina_14_calc_z2p5 — the swapped coefficients (global: 7.089/-1.144, Japan: 5.359/-1.102) match my recollection of CB14 Eqs. 33/34, and Japan now correctly comes out shallower than global for the same Vs30, as expected.
• abrahamson_gulerce_20_calc_z2p5 / parker_20_calc_z2p5 — both were missing the m→km conversion despite the docstring promising km; the added /1000 is a straightforward, correct unit fix.
• Removed regions (AG20-Japan, P20-Cascadia) — confirmed via grep that Z_CALC_MODEL_REGION_MAPPING and these calc functions are currently only exercised by the test suite (oq_wrapper/wrapper.py builds its GMM dispatch table from the openquake.hazardlib.gsim classes directly, not from these estimator functions), so dropping the unverified regions carries no production call-site risk, matching the PR description's claim.
• mod_chiou_young_14_calc_z1p0 (ASK14) — the new global slope (-7.67) and Japan branch (squares, 412 instead of 412.39) are plausible and internally consistent, but I could not independently verify the exact ASK14 Eq. 18/19 coefficients against the paper or OpenQuake's source, since this sandboxed review environment has no outbound network access (WebFetch/WebSearch permission not granted) and shell command execution requires interactive approval that isn't available here, so I couldn't run pytest tests/test_estimations.py myself. This is the one place where I'm relying on the PR author's stated paper cross-check rather than independent verification — I'd suggest confirming the new test_z_matches_openquake test (which diffs against abrahamson_2014._get_z1pt0ref) is green in CI before merging, since that's the strongest available check.
• Test logic in tests/test_estimations.py (erf transition math, clipping ratio math, reference-rock anchors) checks out algebraically against the implementations.

No other issues (style, security, edge-case handling) stood out in the diff.