Skip to content
This repository has been archived by the owner on Apr 24, 2024. It is now read-only.

Adds a property string to ase_to_tensormap #55

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

bananenpampe
Copy link
Collaborator

No description provided.

@bananenpampe bananenpampe requested a review from agoscinski July 17, 2023 14:49
@agoscinski
Copy link
Collaborator

Code looks fine. Lint should get fixed by tox -e format. The documentation error seems to be related to read the docs @PicoCentauri can you have a look at it? Or give me access to the readthedoc page of equisolve?

@bananenpampe
Copy link
Collaborator Author

The stress of an ASE atoms object will be most likely found in the atoms.info dict.
The stress is a 3x3 matrix and ase can only write ie. extxyz files to disk if the .arrays entry has the same number of samples as the number of atoms in the frame

@agoscinski
Copy link
Collaborator

The stress of an ASE atoms object will be most likely found in the atoms.info dict.
The stress is a 3x3 matrix and ase can only write ie. extxyz files to disk if the .arrays entry has the same number of samples as the number of atoms in the frame

Fair argument. I think we do not have yet an example using stress

from attached calculators

Also adds a slightly different testcase that generates random 3x3
symmetric stress matrices
src/equisolve/utils/convert.py Outdated Show resolved Hide resolved
values = [f.info[energy] for f in frames]
else:
energy = "energy"
values = [f.get_potential_energy() for f in frames]
Copy link
Collaborator

Choose a reason for hiding this comment

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

this also needs a check if energy is present right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

mhh, honestly I wanted this to fail since having gradients available but not the property, seems a bit weird to me

Copy link
Collaborator

Choose a reason for hiding this comment

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

you are right, forgot that this just extracts the property

Co-authored-by: Alexander Goscinski <[email protected]>
@@ -9,6 +9,8 @@
import ase
import numpy as np
import pytest
from ase.calculators.calculator import Calculator, all_changes
Copy link
Collaborator

Choose a reason for hiding this comment

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

can this be removed?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants