Skip to content

aimnet2 updates; additional loss options; multi cutoff neighborlists#361

Merged
chrisiacovella merged 29 commits intochoderalab:mainfrom
chrisiacovella:dev-losses
Jun 27, 2025
Merged

aimnet2 updates; additional loss options; multi cutoff neighborlists#361
chrisiacovella merged 29 commits intochoderalab:mainfrom
chrisiacovella:dev-losses

Conversation

@chrisiacovella
Copy link
Copy Markdown
Member

@chrisiacovella chrisiacovella commented Jun 19, 2025

Pull Request Summary

The PR is primarily focused on getting all the odds and ends together for proper utilization of aimnet2.

The changes are outlined below:

Key changes

Notable points that this PR has either accomplished or will accomplish.

  • Aimnet2 returns predicted partial charges
  • Add in post processing operation to allow summation of different energy contributions
  • Adds partial charges into the available properties
  • Partial charges can now be computed in loss functions
  • Multi- cutoff neighbor lists have been reimplemented, now compatible with torch script
  • Documentation for how to properly use AimNet2
  • Add in DFTD3 as a post processing operation

Associated Issue(s)

Pull Request Checklist

  • Issue(s) raised/addressed and linked
  • Includes appropriate unit test(s)
  • Appropriate docstring(s) added/updated
  • Appropriate .rst doc file(s) added/updated
  • PR is ready for review

…s. Also changing naming to reflect quantity type...i.e., electrostatic_energy is now per_system_electrostatic_energy
…now return per_atom_charge predictions/targets. Dipole moment is also separated from total_charge function (since we can do dipole moment loss independent of total_charge loss).
…re the other pairlistdata instances for each cutoff
@chrisiacovella chrisiacovella added enhancement New feature or request WIP Work in Progress refactoring Improve the quality of the code without functional changes labels Jun 19, 2025
@chrisiacovella
Copy link
Copy Markdown
Member Author

I'll note this is still partially a WIP. I still need to implement some CI tests for the multi-cutoff neighbor lists and add in the DFTD3 post-processing routine (although I might end up skipping that for this PR and do that in a separate one).

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 19, 2025

Codecov Report

❌ Patch coverage is 91.98606% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.98%. Comparing base (6d3272c) to head (fb2a8da).
⚠️ Report is 271 commits behind head on main.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR integrates per-atom partial charge support, extends loss functions to include charge errors, and refactors neighborlist routines to support multiple cutoffs alongside adding DFTD3-based dispersion and energy‐summation postprocessing.

Key changes:

  • Added partial_charge flow through PropertyNames, Metadata, BatchData, training, and Loss.
  • Refactored NeighborListFor* to return PairlistOutputs with local, vdw, and electrostatic cutoffs.
  • Introduced DispersionPotential, SumPerSystemEnergies, and updated PostProcessing for new per-system energy terms.

Reviewed Changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
modelforge/utils/prop.py Added partial_charge to PropertyNames and Metadata.
modelforge/train/training.py Updated CalculateProperties to handle per-atom and total charges; renamed helper methods.
modelforge/train/losses.py Added PerAtomChargeError and updated Loss to include per_atom_charge.
modelforge/potential/processing.py Added SumPerSystemEnergies, DispersionPotential, and multi-cutoff postprocessing; contains a key‐naming bug.
modelforge/potential/neighbors.py Refactored neighborlist classes to return PairlistOutputs; updated internal logic for multiple cutoffs.
modelforge/tests Updated tests for .local_cutoff API and added test_per_atom_charge_loss.
Comments suppressed due to low confidence (2)

modelforge/potential/processing.py:435

  • The key electrostatic_r_ij] contains an extra bracket and won't match downstream usage (electrostatic_r_ij). Remove the trailing ] inside the string.
        Returns

modelforge/potential/processing.py:336

  • The forward method in SumPerSystemEnergies is defined inside __init__, making it a local function rather than a class method. Move def forward(...) out of __init__ and indent it at the class level.
        def forward(self, data: Dict[str, torch.Tensor]) -> Dict[str, torch.Tensor]:

Comment thread modelforge/tests/test_training.py
Comment thread modelforge/train/losses.py
Comment thread docs/datasets.rst Outdated
Comment thread modelforge/tests/test_dataset.py
Comment thread modelforge/utils/prop.py Outdated
@chrisiacovella chrisiacovella merged commit c48ceac into choderalab:main Jun 27, 2025
17 checks passed
@chrisiacovella chrisiacovella deleted the dev-losses branch August 27, 2025 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request refactoring Improve the quality of the code without functional changes WIP Work in Progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants