Skip to content

Fix quadrature noise in linking number diagnostic - #63

Open
rogeriojorge wants to merge 13 commits into
mainfrom
fix/integer-linking-number
Open

rogeriojorge wants to merge 13 commits into
mainfrom
fix/integer-linking-number

Conversation

@rogeriojorge

@rogeriojorge rogeriojorge commented Aug 24, 2026 •

Copy link
Copy Markdown
Member

Round each coil pair’s absolute Gauss integral to its integer linking number, matching SIMSOPT. Stop gradients through this topological diagnostic and mask padded quadrature entries without changing the physical denominator. Optional downsampling reduces pairwise work.

Tests cover Hopf links, unlinked circles, orientation, symmetry copies, uneven quadrature blocks, and zero gradients. Mandatory NumPy reference tests and optional SIMSOPT parity tests verify each pair and the aggregate. Linking number remains undefined for intersecting curves.

Recorded validation: reference/SIMSOPT parity and the 99-test suite passed (one expected failure). Python 3.10–3.12 CI passes. No production dependency is added.

@codecov

codecov Bot commented Aug 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
essos/objective_functions.py 99.08% <100.00%> (+0.33%) ⬆️

... and 1 file with indirect coverage changes

@rogeriojorge

Copy link
Copy Markdown
Member Author

Added SIMSOPT-compatible downsample support in 0fd8379. The integration step is scaled by the factor, invalid/non-divisible values are rejected, and the existing blockwise path operates on the reduced point set. Tests compare raw per-pair integrals and exact integer classifications against both the NumPy translation of SIMSOPT C++ and installed SIMSOPT for factors 1, 2, 3, 4, 5, and 6, covering expanded unlinked circles and a Hopf link. Full local suite: 99 passed, 1 xfailed. Benchmark for 56 expanded coils / 80 source points: 6.61 ms at downsample=1, 1.33 ms at 2, and 0.45 ms at 8; all report linking number 0.

@rogeriojorge
rogeriojorge force-pushed the fix/integer-linking-number branch from 0fd8379 to e6d8005 Compare September 27, 2026 04:43

@eduardolneto eduardolneto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The changes to add restructure and add extra compatibility with simsopt equivalent calculations are good. However, should we find a way to get an alternative loss function that is differentiable?

@rogeriojorge
rogeriojorge force-pushed the fix/integer-linking-number branch from a32d831 to da43b57 Compare October 5, 2026 18:19
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.

2 participants