Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OptimizableBatch and stress relaxations #718

Merged
merged 126 commits into from
Dec 3, 2024
Merged

OptimizableBatch and stress relaxations #718

merged 126 commits into from
Dec 3, 2024

Conversation

lbluque
Copy link
Collaborator

@lbluque lbluque commented May 28, 2024

Add functionality to include cell relaxations in structural relaxations for models that implement stress prediction. This PR also enables batch calculations with ASE optimizers.

  • Extends the OCPCalculator to enable stress predictions, and relaxations with ASE filters.
  • Extends core.common.relaxation.ml_relax to enable batch relaxations including stress.
  • Includes a new class OptimizableBatch that can be used with ASE optimizers or with the existing LBFGS implementation
  • Includes a UnitCellOptimizableBatch to allow batch stress relaxations
  • Includes a ExpCellOptimizableBatch to allow batch stress relaxations will do this in a separate PR

TODO

  • Unit tests (missing tests with unit cell relaxation, pending models that can predict stress)

@lbluque lbluque marked this pull request as draft May 28, 2024 19:06
@zulissimeta zulissimeta self-requested a review November 22, 2024 01:41
zulissimeta and others added 2 commits November 21, 2024 17:43
# Conflicts:
#	src/fairchem/core/trainers/base_trainer.py
rayg1234
rayg1234 previously approved these changes Dec 2, 2024
@lbluque lbluque enabled auto-merge December 3, 2024 01:50
zulissimeta
zulissimeta previously approved these changes Dec 3, 2024
Copy link
Collaborator

@zulissimeta zulissimeta left a comment

Choose a reason for hiding this comment

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

Approving with trivial update to sphinx autoapi and @rayg1234 's review of everything else!


# system level model predictions have different shapes than expected by ASE
ASE_PROP_RESHAPE = MappingProxyType(
{"stress": (-1, 3, 3), "dielectric_tensor": (-1, 3, 3)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a little strange to have these hard-coded. What should a user do implementing new properties?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree this is strange, but it's the simplest solution I thought of. We can always generalized this to simply "tensor of rank X" but in that case we need to have a data structure that provides that information for each model output.

We could use the properties interface and properties defined in ASE to clean this up, but I would suggest doing that in a new PR:
https://gitlab.com/ase/ase/-/blob/master/ase/outputs.py

src/fairchem/core/common/relaxation/__init__.py Outdated Show resolved Hide resolved
@lbluque lbluque added this pull request to the merge queue Dec 3, 2024
@lbluque lbluque removed this pull request from the merge queue due to a manual request Dec 3, 2024
@lbluque lbluque enabled auto-merge December 3, 2024 18:07
@lbluque lbluque added this pull request to the merge queue Dec 3, 2024
Merged via the queue into main with commit 3f695ef Dec 3, 2024
10 checks passed
@lbluque lbluque deleted the stress-relaxations branch December 3, 2024 19:56
misko pushed a commit that referenced this pull request Jan 17, 2025
* remove r_edges, radius, max_neigh and add deprecation warning

* edit typing and dont use dicts as default

* use super() and remove overkill deprecation warning

* set implemented_properties from config

* make determine step a method

* allow calculator to operate on batches

* only update if old config is used

* reshape properties

* no test classes in ase calculator

* yaml load fix

* use mappingproxy

* expressive import

* remove duplicated code

* optimizable batch class for ase compatible batch relaxations

* fix optimizable batch

* optimizable goodies

* apply force constraints

* use optimizable batch instead and remove torchcalc

* update ml relaxations to use optimizable batch correctly

* force_consistent check for ASE compat

* force_consistent check for ASE compat

* check force_consistent

* init docs in lbfgs

* unitcellfilter for batch relaxations

* ruff

* UnitCellOptimizable as child class instead of filter

* allow running unit cell relaxations

* ruff

* no grad in run_relaxations

* make batched_dot and determine_step methods

* imports

* rename to optimizableunitcellbatch

* allow passing energy and forces explicitly to batch to atoms

* check convergence in optimizable and allow passing general results to atoms_from_batch

* relaxation test

* unit tests

* move update mask to optimizable

* use energy instead of y

* all setting/getting positions and convergence in optimizable

* more (unfinished) tests

* backwards compatible test

* minor fixes

* code cleanup

* add/fix tests

* fix lbfgs

* assert using norm

* add eps to masked batches if using ASE optimizers

* match iterations from previous implementation

* use float64 for forces

* float32

* use energy_relaxed instead of y_relaxed

* energy_relaxed and more explicit error msg

* default to batch_size 1 if not set in config

* keep float64 training

* rename y_relaxed -> energy_relaxed

* rm expcell batch

* convenience commit from no_experimental_resolve

* use numatoms tensor for cell factor

* remove positions tests (wrapping atoms gives different results)

* allow wrapping positions in batch to atoms

* fix test

* wrap_positions in batch_to_atoms

* take a2g properties from model

* test lbfgs traj writes

* remove comments

* use model generate graph

* fix cell_factor

* fix using model in ddp

* fix r_edges in OCPcalculator

* write initial and final structure if save_full is false

* check unique atoms saved in trajectory

* tighter tol

* update ASE release comment

* remove cumulative mask option

* remove left over cumulative_mask

* fix batching when sids as str

* do not try to fetch energy and forces if no explicit results

* accept Path objects

* clean up setting defaults

* expose ml_relax in relaxation

* force set r_pbc True

* make relax_opt optional

* no ema on inference only

* define ema none to avoid issues

* lower force threshold to make sure test does not converge

* clean up exception msg

* allow strings in batch

* remove device argument from lbfgs

* minor cleanup

* fix optimizable import

* do not pass device in ml_relax

* simplify enforce max neighbors

* fix tests (still not testing stress)

* pin sphinx autoapi

* typo in version

---------

Co-authored-by: zulissimeta <[email protected]>
Co-authored-by: Zack Ulissi <[email protected]>
Former-commit-id: dc4e598eef45c70e46549e00857968a40f784602
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request minor Minor version release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants