Skip to content

perf: build CompoundGeometry.geom lazily#359

Merged
mortenengen merged 2 commits into
fib-international:devfrom
magnusfjeldolsen:perf/lazy-svg-geom
Jun 5, 2026
Merged

perf: build CompoundGeometry.geom lazily#359
mortenengen merged 2 commits into
fib-international:devfrom
magnusfjeldolsen:perf/lazy-svg-geom

Conversation

@magnusfjeldolsen

@magnusfjeldolsen magnusfjeldolsen commented May 29, 2026

Copy link
Copy Markdown
Contributor

Closes #358

CompoundGeometry.__init__ eagerly built self.geom — the MultiPolygon
used only by _repr_svg_ for display — by buffering every point geometry into
a circle. This ran on every construction, including every rotate() call
inside the equilibrium solvers, even though the result is only ever needed for
SVG display.

This PR defers it to a lazy geom property computed on first access (cached in
_geom). Behaviour on read is unchanged: geom returns the same MultiPolygon
and _repr_svg_ is untouched.

Performance (medium-complexity RC section, best of 5, vs dev)

  • Marin integrator: bending capacity ~1.3×, moment–curvature ~1.4×, N–M ~1.4×
    (marin reconstructs geometry once per iteration)
  • Fiber integrator: ~1.0×
    (fiber reconstructs geometry only once per calculation)

Risk assessment

Low, in my assessment. No material state, no public API change. The lazy property is the
only writer of _geom; _repr_svg_ is the only reader of geom. All
construction paths (__init__, rotate/translate/mirror/__add__/
__sub__/from_geometry) initialise _geom.

Tests

Full geometry + section suite passes; ruff format and ruff check clean.
Added test_compound_geometry_lazy_geom covering lazy build, correct contents,
caching, and recomputation after a transform.

self.geom (the MultiPolygon used only by _repr_svg_ for display) was built
in __init__ by buffering every point geometry into a circle on every
construction -- including every rotate() inside the equilibrium solvers.
Defer it to a lazy `geom` property computed on first access.

Standalone speed-ups vs main (medium section, best of 5):
  marin: bending 1.5x, moment-curvature 1.4x, N-M 1.4x
  fiber: ~1.0-1.1x (fiber rarely reconstructs geometry)
All 5494 geometry + section tests pass.
Assert that `geom` is built on first access (not in __init__), contains one
polygon per surface geometry plus a buffered circle per point geometry, is
cached on repeated access, and is recomputed by a transformed copy.
@mortenengen mortenengen self-requested a review June 1, 2026 05:56
@mortenengen mortenengen added the enhancement New feature or request label Jun 1, 2026
@mortenengen mortenengen moved this to Needs review 📣 in PR tracker Jun 1, 2026

@talledodiego talledodiego left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally agree with this, thanks!

@mortenengen mortenengen moved this from Needs review 📣 to Under review 👀 in PR tracker Jun 5, 2026

@mortenengen mortenengen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this useful improvement!

@mortenengen mortenengen merged commit 4b0ce13 into fib-international:dev Jun 5, 2026
4 checks passed
@github-project-automation github-project-automation Bot moved this from Under review 👀 to Done 🚀 in PR tracker Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done 🚀

Development

Successfully merging this pull request may close these issues.

3 participants