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

Fix data model #117

Merged
merged 9 commits into from
Dec 10, 2024
Merged

Fix data model #117

merged 9 commits into from
Dec 10, 2024

Conversation

ElliottKasoar
Copy link
Contributor

@ElliottKasoar ElliottKasoar commented Nov 29, 2024

Introduces various fixes, mostly relating to converting data to and from Atoms.calc.results:

  • Updates ASE to 3.23+, due to changes in how various quantities are stored (e.g. previously, energy would also be stored in Atoms.info)
    • I think all changes are still valid for 3.22.1, but the tests would fail due to differences in how values are stored.

from_atoms:

  • Ensures keys are not duplicated between info and results, which previously caused errors when pushing/uploading with store_calc.
  • Removed adding results to info and array keys unnecessarily
    • This also fixes a bug where, for example arrays_keys.update(key) splits key into individual letters, rather than adding the entire key to the set

to_ase:

  • Fixes setting keys (and when they are set) when dealing with calculated results, to avoid mislabelling/missing key errors.

Copy link
Member

@stenczelt stenczelt left a comment

Choose a reason for hiding this comment

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

Great job, I've added two more test cases when exploring your code and it seems you may have solved #80 as well here. See #118 as well for some new problems I've encountered, but that's unrelated to this problem apart from touched on by one of my tests - let's address that separately.

@stenczelt stenczelt merged commit 4f1b0f6 into libAtoms:master Dec 10, 2024
4 checks passed
@ElliottKasoar ElliottKasoar mentioned this pull request Jan 3, 2025
@ElliottKasoar ElliottKasoar deleted the update-ase branch January 3, 2025 14:14
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