Skip to content

fix: world locations for display, export and intrusion classification; deprecate rescale and scale, stage S4 (spec 005) - #401

Merged
lachlangrose merged 18 commits into
masterfrom
feat/005-s4-world-locations
Nov 5, 2026
Merged

lachlangrose merged 18 commits into
masterfrom
feat/005-s4-world-locations

Conversation

@lachlangrose

@lachlangrose lachlangrose commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

Summary

Stage S4 of spec 005 (coordinate and fault-frame correctness): display, data export and intrusion classification use the stored world locations (FR-022), and GeologicalModel.rescale and scale are deprecated (FR-033). The spec, plan and tasks are in specs/005-coordinate-frame-correctness/. All 18 S4 tasks (T051 to T068) are done. S5 and S6 are not planned yet.

Changes

  • Display and export (FR-022): GeologicalFeature.get_data no longer calls model.rescale on the builder rows. The rows are world data, so the call moved them by the local origin and rotated them a second time. With a local origin of (100,200,300) the point (102,203,304) was shown at (202,403,604). Loop2DView.add_data and ModelExporter.save now get the world locations.
  • Intrusion classification (FR-022): a new private method IntrusionFrameBuilder._network_model_values evaluates the model at the world X, Y and Z of the network data. set_intrusion_steps_parameters uses it. Before, the classification read the unit at a shifted point.
  • Deprecation (FR-033): GeologicalModel.rescale and scale keep their behaviour and signature, and emit one DeprecationWarning that names bounding_box.reproject and bounding_box.project. The tier stays "stable". COMPAT.md has two new rows in the deprecation table (earliest removal: 2 minor releases after the first release that warns). ROADMAP.md and API.md record it.
  • Callers moved first: the four discarded model.rescale calls in exporters.py are gone (they did nothing; three if real_coords: lines that only held them are gone too). ModelExporter.get_block_model uses bounding_box.reproject. The faulted intrusion example no longer applies a second conversion.
  • No fault-centre change was needed: the centre is a world point since S1, and the 2D viewer draws it as it is. A coverage test records this.
  • stacklevel=3 in both warnings, because the @public_api wrapper adds a frame. A test checks that the warning names the caller file.
  • scale has always copied its input, so inplace=True never changed the caller array. S4 keeps this (FR-033: keep behaviour).

Behaviour change

get_data() locations change for models with a non-zero local origin or a rotation: they are now the world locations. With an identity frame nothing changes. Intrusion models in a translated frame can classify their network data differently, because the classification is now read at the right points. COMPAT.md has a migration notice. Users who corrected the display by shifting their data now see a shifted display.

Test plan

  • ruff check LoopStructural packages: pass
  • pytest packages/loop_common/tests packages/loop_interpolation/tests: 912 passed, 47 skipped, 2 xfailed
  • pytest (root), run with pyevtk on the path: 2066 passed, 4 skipped, 1 failed. The failure was test_exporters.py::test_write_vol_gocad_is_broken_for_real_bounding_box. It also fails without the S4 changes. Two classes are named LoopValueError and pytest.raises matched the wrong one. The last commit fixes the test (it now accepts both classes). The file passes (13 passed). I did not re-run the full root suite after this commit and after the end-review fix.
  • New tests: tests/unit/modelling/test_world_location_consumers.py (display, export, intrusion classification, call-site test of set_intrusion_steps_parameters, 2D fault ellipse, rescale and scale warnings, no library caller warns). Before the fix, 18 of 19 cells of the first three tests failed (the passing cell is the identity frame).
  • Wave reviews pass (9). The end review (Opus) found one major gap: no test ran the real call site of the intrusion classification. A new test covers it, and two mutations of the call site make it fail. The recheck passed.
  • Rollback check: see verification.md ("S4 rollback checks").

Notes

  • 4 minor review findings stay open (listed in verification.md and run-state.json): the ellipse test checks only against fault.fault_centre; one importorskip("pyevtk") sits in the middle of a test; stale line numbers in a grep note.
  • black --check reports 3 library files that were not Black-clean before S4.
  • The 2D viewer test is skipped when loopstructuralvisualisation is not on the path; the exporter tests skip without pyevtk.
  • Spec 005 S5 and S6 remain.

lachlangrose and others added 18 commits November 5, 2026 15:56
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
…5 S4 (wave 25)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
… (wave 26)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
…5 S4 (wave 27)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
…nd intrusion classification; deprecate rescale and scale (005 S4, wave 28)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
…ion for 005 (wave 29)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
…t; name Loop2DView.add_data in the S4 docs

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XADRvJ98bJ8Ey8Rw98F3j1
@lachlangrose
lachlangrose merged commit 62c420e into master Nov 5, 2026
8 checks passed
@lachlangrose
lachlangrose deleted the feat/005-s4-world-locations branch November 5, 2026 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant