Skip to content

Compose models for allegro-pol simulation#537

Draft
CompRhys wants to merge 1 commit intomainfrom
allegro-pol
Draft

Compose models for allegro-pol simulation#537
CompRhys wants to merge 1 commit intomainfrom
allegro-pol

Conversation

@CompRhys
Copy link
Copy Markdown
Member

@CompRhys CompRhys commented Apr 8, 2026

Adds SerialSumModel to allow for allegro-pol to make predictions and pass to subsequent model in single sum model.

@CompRhys CompRhys changed the base branch from main to extensible-extras April 8, 2026 19:40
Copy link
Copy Markdown
Collaborator

@orionarcher orionarcher left a comment

Choose a reason for hiding this comment

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

Not sure I fully understand the purpose of UniformPolarizationModel or when it should be used, which probably betrays either my ignorance or the need for slightly more documentation. Implementation seems good aside from a few docs and interface suggestions.

@CompRhys CompRhys force-pushed the allegro-pol branch 2 times, most recently from 82c6af5 to bcc58c2 Compare April 8, 2026 20:30
@TorchSim TorchSim deleted a comment from orionarcher Apr 8, 2026
@TorchSim TorchSim deleted a comment from orionarcher Apr 8, 2026
@TorchSim TorchSim deleted a comment from orionarcher Apr 8, 2026
@CompRhys CompRhys marked this pull request as draft April 8, 2026 21:08
@CompRhys CompRhys added the feature Entirely new features, not improvements to existing ones label Apr 9, 2026
@CompRhys
Copy link
Copy Markdown
Member Author

CompRhys commented Apr 9, 2026

Not sure I fully understand the purpose of UniformPolarizationModel or when it should be used, which probably betrays either my ignorance or the need for slightly more documentation. Implementation seems good aside from a few docs and interface suggestions.

So the aim of the contribution is that if you run a simulation of polarizable material in an electric field the additional terms are just a sum contribution to the overall forces and energy in the small field limit that models like allegro-pol were trained for.

The challenge was that with the first SumModel I added last week the can't handle the case where the first model generates extras that are used by the second. The serial sum model here allows for that [h(x) = f(x) + g(x)) vs h(x) = p(x) + q(x, r(x))]. So if we call allegro-pol to get the polarization and born charges then we can take those as the input to the UniformPolarizationModel and get the energy contributions from the uniform electric field.

The UniformPolarizationModel name comes from the fact that we should also have a SpatialPolarizationModel which would let you vary the E field in the cell. The maths there is more involved because of image charges and PBC. That can be a later addition.

Base automatically changed from extensible-extras to main April 10, 2026 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Entirely new features, not improvements to existing ones

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants