Implementation of compliance elements and reusable internal coordinate infrastructure by lmseidler · Pull Request #1446 · grimme-lab/xtb

lmseidler · 2026-09-15T09:32:45Z

The goal of this PR is to introduce the calculation of compliance elements from the Cartesian Hessian. In the process I'll also implement a robust internal coordinate implementation, which can later be used to replace redundant code in different modules (intmodes, geosum) in a separate PR.

Edit: there is also potential to further reduce redundant code in model Hessian infrastructure but this needs some special internal coordinate generation rules and is beyond the scope of this PR. Added a TODO though.

Copilot

Copilot review overview

🟡 Changes recommended

Pure-procedure compilation blockers and correctness/coverage gaps remain in the new internal-coordinate and compliance paths.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 · 3

Open (6)

ERROR STOP is invalid in pure get_bmatrix · New
Linear coord_angle dispatch omits required bend coordinates · New
ERROR STOP is invalid in pure new_redundant · New
print_compl applies incorrect mass-unit conversion · New
Empty neighbour list causes maxval on zero-sized arrays · New
GFN1 linear-water regression test is not registered · New

What changed in this PR

This PR adds compliance-matrix calculation from Cartesian Hessians, reusable redundant internal-coordinate infrastructure, and refactors model-Hessian configuration.

Changes:

Adds covalent neighbour graphs, redundant coordinates, and Wilson B-matrix assembly.
Integrates compliance reporting into numerical Hessian workflows.
Updates model-Hessian constructors and expands regression tests.

File	Description
`test/unit/test_model_hessian.f90`	Updates model-Hessian constructor tests.
`test/unit/test_hessian.f90`	Adds compliance and internal-coordinate tests.
`test/unit/test_bmatrix.f90`	Expands linear-bend finite-difference tests.
`src/type/neighbourlist.f90`	Adds covalent neighbour generation.
`src/type/calculator.f90`	Uses configured Swart Hessian construction.
`src/optimizer.f90`	Updates model-Hessian allocation and calls.
`src/model_hessian/type.f90`	Simplifies the model-Hessian interface.
`src/model_hessian/{swart,lindh,internal,gff}.f90`	Adds constructors and configuration-backed computation.
`src/meson.build`	Registers internals and compliance sources.
`src/internals/*`	Adds graph and redundant-coordinate infrastructure.
`src/hessian.F90`	Invokes compliance reporting after numerical Hessians.
`src/compliance.f90`	Implements compliance calculation and output.
`src/CMakeLists.txt`	Registers new sources.
`src/bmatrix.f90`	Adds generic B-matrix assembly and linear-bend helpers.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

thfroitzheim · 2026-09-20T18:57:25Z

+   type(graph_type), intent(in) :: graph
+   !> Cartesian reference coordinates, dimension (3, graph%n), in the length
+   !> unit the coordinate values are reported in
+   real(wp), intent(in) :: xyz(:, :)


Wouldn't it be better to take a structure object in here instead of moving around its components?

to keep the signature as simple as possible and to minimize dependencies (this method only needs the cartesian coords, no need to import also the molecule type) i think it's preferable to pass only the single attribute.