Skip to content

Commit 880e4aa

Browse files
authored
Merge pull request #45 from brmather/optimiser-least-squares
Fit with least_squares instead of L-BFGS-B
2 parents 59f2b75 + 3bbed8e commit 880e4aa

3 files changed

Lines changed: 484 additions & 53 deletions

File tree

CLAUDE.md

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ returns a bare number without one is a pre-v2 remnant.
1515
## Commands
1616

1717
```bash
18-
pytest # 70 tests, ~40 s
19-
pytest -m "not slow" # 67 tests, ~26 s -- skips the calibration tests that
18+
pytest # 111 tests, ~20 s
19+
pytest -m "not slow" # 108 tests, ~12 s -- skips the calibration tests that
2020
# fit a few hundred realisations
2121
pytest tests/test_tanaka.py -q
2222
```
@@ -78,6 +78,25 @@ depth.
7878
**Depths are positive downwards.** `optimise` returns depths, not the negative
7979
gradients the fits produce.
8080

81+
**Every Bouligand fit goes through `_fit`, which is `least_squares`, not
82+
`minimize`.** Do not put L-BFGS-B back: it needs **2012** evaluations of the
83+
forward model per vertex where trust-region reflective needs 359, about 3x the
84+
CPU. `_fit` also supplies the two exact Jacobian columns (`dr/dzt = -2k/sigma`,
85+
`dr/dC = 1/sigma`); `beta` enters through the *order* of a Bessel function and
86+
`dz` costs the same analytically as by difference, so those two stay numerical.
87+
88+
**Pin the BLAS before timing anything.**
89+
90+
```bash
91+
OMP_NUM_THREADS=1 OPENBLAS_NUM_THREADS=1 MKL_NUM_THREADS=1 python bench.py
92+
```
93+
94+
L-BFGS-B calls a threaded BLAS whose workers spin-wait. Unpinned on a loaded
95+
machine it looks 20x to 400x slower than TRF depending on contention, and
96+
`time.process_time` makes it worse by charging every spinning thread — a
97+
"3.1 s CPU" fit whose wall clock was 1.7 s. Both numbers were wrong; the truth
98+
is 3x. Counting `residuals` calls is the measurement that does not lie.
99+
81100
**Tanaka bands have no defaults, deliberately.** Each straight-line limit holds
82101
only over part of the spectrum: the `zt` band needs wavelengths shorter than
83102
~4x the source thickness, the `z0` band needs `|k|d << 1`. Violating either
@@ -202,6 +221,17 @@ left in place, which makes a correct `MANIFEST.in` look broken.
202221

203222
## Known defects
204223

224+
- **`profile` reports one basin of a multimodal deviance.** The scan walks
225+
outward from the best node to the first threshold crossing, so where the
226+
misfit has two minima it covers the one around the best node and never sees
227+
the other, and the interval can then exclude the fitted value. Measured on
228+
synthetics at a 200 km window, 2 of 20 intervals did. It does **not** appear
229+
in the regime that matters: over cached EMAG2 spectra in `~/Global_CPD`
230+
three window sizes from 1000 to 4000 km, three targets, twelve mesh vertices
231+
— 0 of 108 intervals excluded their estimate. Band limiting and the prior
232+
pinning `zt` between them seem to remove it. Worth knowing if you profile a
233+
small unconstrained synthetic; not worth guarding against.
234+
205235
- **`install_documentation()` fails for an installed package.** It is still
206236
advertised in the README, but `[tool.setuptools] packages =
207237
["pycurious"]` installs only the package directory, and `Examples/` sits at the

0 commit comments

Comments
 (0)