Version 3 - Failsafe Polyhedral Gravity Modeling & Polyhedron Class

[schuhmaj]

Changelog

PyPi Release Release Candidate 3.0rc3

Published on PyPi as Pre-Release 3.0rc3 and installable via explicit versioning:

pip install polyhedral-gravity==3.0rc3

Resolves Issues

• Resolves Check validity of the polyhedra before computing gravity fields? #34

The changes introduced with this PR make it impossible to create an invalid Polyhedron (if not explicitly requested). Hence, the gravity model guarantees correct results according to Tsoulis' formulation.
We are also NOT dependent anymore on INWARDS or OUTWARDS pointing normals. Both cases can be handled.

The changes are quite extensive - especially due to changes to the initial architectural concept being necessary. Below, you'll find a list of interesting files for the review.

Rational of the Changes

The problem with #34 is that it requires adding at least two options: one to disable/ enable checking and one to hand over the plane unit normal orientation (now only referred to as orientation). Hence, extending the evaluate method would lead to a method with six parameters.
Further, the orientation is actually correlated to mesh vertices and faces.
There is no good way to integrate this in the current method without:

• a polyhedral definition that is split across multiple parameters that actually belong together
• a method that violates the single responsibility principle, a.k.a. the gravity model also doing mesh checking

The new Way with a fully fleshed Polyhedron

The PR fundamentally reworks the input handling and separates gravity evaluation & mesh handling by promoting the Polyhedron to an actual class with behavior rather than just a data container.
This also paves the way for (native on the C++ side) heterogeneous density evaluation in the future.

New design approach:

One first creates a Polyhedron from a polyhedral_source (file input list or vertices/ faces).
It is NOT possible to create an invalid polyhedron leading to wrong results in the gravity evaluation (as long as integrity_check != PolyhedronIntegrity.DISABLE).
This is also the magic of the new implementation:

• AUTOMATIC (default): Informs the user about $O(n^2)$ runtime cost and raises ValueError if orientation is different than specified or degenerated faces appear (first-time user)
• VERIFY: Like AUTOMATIC and raises an exception, but without the info-message to stdout
• HEAL: No exception; the mesh is repaired automatically to make the normal orientation consistent (still raises if degenerated faces appear)
• DISABLE: All checks disabled, no runtime cost (experienced user), allows the construction of invalid polyhedrons

--> Details in docs, see in rendered rst

Example

Create a Polyhedron:

from polyhedral_gravity import Polyhedron, GravityEvaluable, evaluate, PolyhedronIntegrity, NormalOrientation

polyhedron = Polyhedron(
    polyhedral_source=(vertices, faces),
    density=density,
    normal_orientation=NormalOrientation.OUTWARDS, # Possible values OUTWARDS (default) or INWARDS
    integrity_check=PolyhedronIntegrity.VERIFY,  # Possible values AUTOMATIC (default), VERIFY, HEAL, DISABLE
)

The evaluate method only takes an object of kind Polyhedron (so no wrong input possible/ separation of concerns):

sol = evaluate(
    polyhedron=polyhedron,
    computation_points=points,
    parallel=True,
)

Or with caching, the GravityEvaluable:

evaluable = GravityEvaluable(polyhedron=polyhedron)
sol = evaluable(
    computation_points=points,
    parallel=True,
)

Major Changes and Features

For review interesting:

• src/polyhedralGravityPython/PolyhedralGravityPython.cpp
• src/polyhedralGravity/model/Polyhedron.h/cpp
• test/model/PolyhedronTest.cpp
• test/model/PolyhedronBigTest.cpp
• test/python/test_polyhedron.py
• test/python/test_gravity_model.py
• The shiny documentation

The rest are nearly only movements or changes required due to the refactored interface.

C++ Library

• Add enum NormalOrientation, which makes the Mesh Orientation more expressive
  • We don't use clockwise/ anti-clockwise ordering like many mesh tools, as their definition also depends on the utilized coordinate system
  • OUTWARD and INWARD pointing normals are expressive independent of the chosen reference frame (and also the only thing we need to know for Tsoulis et al.'s model)
• Move MeshChecking namespace into Polyhedron
  • Add function getPlaneUnitNormalOrientation() which returns
    • the majority plane unit normal orientation of the faces' vertices (INWARDS or OUTWARDS)
    • a list of faces violating the majority orientation
    • Hence, an empty list means that the mesh is fine with the respective orientation, and a set with elements indicates a syntax error/ the user is obligated to check what's wrong (even though we could theoretically infer the mistake and fix it)
• Move density from GravityEvaluable into Polyhedron
• Add Enum PolyhedronIntegrity to select mesh checking modes on Polyhedron construction.
• New test cases for the Polyhedron class regarding mesh checking

Python Interface

• All new Documentation
• NEW Expose classes
  • Polyhedron
    • with read-only access to vertices & faces (including __getitem__ convenience access to resolved faces with vertex coordinates)
    • with read-write access to density
    • with check_normal_orientation() for debugging what's wrong with the polyhedron (only viable if constructed with PolyhedronIntegrity.DISABLE as otherwise a wrong polyhedron is not constructible)
  • NormalOrientation
  • PolyhedronIntegrity
• CHANGED Refactored signatures (these only take Polyhedron from now)
  • GravityEvaluable
  • evaluate
• REMOVED
  • utility submodule
    • Function read replaced by accesible properties of a polyhedron polyhedron.vertices and polyhedron.faces
    • Function check_mesh replaced by Polyhedron's automatic capabilities + its debug method (just in case)
• New test cases for the Polyhedron class regarding mesh checking

Other

• 🥳 AUTOGENERATED Python Interface Documentation from the C++ pybind11 definition, finally!!! 🥳
• NEW CI/CD builds PyPi wheels for MacOS ARM64: Given the result of https://github.com/esa/polyhedral-gravity-model/actions/runs/8756615419, building for macOS ARM64 works well using the Apple Clang compiler instead of gcc@12 in the CI/CD of the GitHub Runner. However, the aarch64 Linux build doesn't work in the CI/CD --> Hence, I will build this locally/ publish it using my machine as already done for version 2.1
• Update Docs to the new way of doing things
• Update jupyter notebook and scripts
• Increment version in setup.py
• Add the current (CLion default) C++ code style as .clang-format
• Improve the CI to also run on windows & macOS
• Improve our PyPi page (improving the setup.py)

[gomezzz]

I think the minimal example needs updating with this, right? At least it fails for me now :P

Never mind you already did update it :) Was on the wrong branch

[gomezzz]

Curious, github ate my "Submit review" click yesterday it seems. Some minor comments / suggestions.

Nice work!

One open question for me is whether we checked performance is on the same scale as before? I don't see particular cause for worry, just to doublecheck?

[schuhmaj]

Curious, github ate my "Submit review" click yesterday it seems. Some minor comments / suggestions.

Hmm, dark magic going on. That happened to me recently with a comment.

Nice work!

Thanks!

One open question for me is whether we checked performance is on the same scale as before? I don't see particular cause for worry, just to doublecheck?

@gomezzz Thank you very much for the review 😀 I included a performance comparison in the comments above (with an explanation). We're faster or equal than version v2.1 (the evaluate benefits from the Polyhedron class being exposed, as construction is now "outsourced")

For quick reference, here again:

[gomezzz]

💪