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

Dataset object for v1 #135

Merged
merged 10 commits into from
Nov 14, 2023
Merged

Dataset object for v1 #135

merged 10 commits into from
Nov 14, 2023

Conversation

ipcamit
Copy link

@ipcamit ipcamit commented Nov 9, 2023

Summary

Include a summary of major changes in bullet points:

  • Supports ase-io for inputting multiple xyz configs per file
  • Supports colabfit for io directly from mongo-db instance
  • Minor updates regarding file io from softlinked files
  • Test for ase-io

Additional dependencies introduced (if any)

  • ASE: for file io

dependabot bot and others added 6 commits August 1, 2023 20:25
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 4.6.1 to 4.7.0.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@v4.6.1...v4.7.0)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…/actions/setup-python-4.7.0

Bump actions/setup-python from 4.6.1 to 4.7.0
…et can now follow symlinks. Updated tests, and install ASE at test time for Dataset-ase loader tests.
@mjwen mjwen self-requested a review November 10, 2023 15:15
Copy link
Collaborator

@mjwen mjwen left a comment

Choose a reason for hiding this comment

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

@ipcamit I've left some comments. Would be happy to discuss them.

kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
@ipcamit
Copy link
Author

ipcamit commented Nov 13, 2023

Pushing new changes as per suggestions:

  1. make ASE import direct, and not conditional
  2. Fixed missing docs
  3. Changed Configuration to cls in classmethod
  4. Now that ase is a dependency, moved string based typehints to Atoms
  5. Added stress setter
    6 Added metadata field to Configuration for colabfit dataset properties
  6. New dataset initialization methods
  7. fixed tests to reflect new dataset interface

On new dataset initialization:
Now Dataset object supports three classmethods, from_colabfit, from_path, and from_ase, to initialize the dataset. Similarly, it has three additional methods called add_from_colabfit, add_from_path, and add_from_ase, to extend the datasets from these sources.

The main difference between *from_path and *from_ase is that ASE function can also accept list of ase.Atoms object to initialize the dataset. Another major difference is that while ase accepts Voigt notation in stress, it usually expects the xyz file to have full 9-member matrix. Therefore we have two functions, from_path, and from_ase, where from_ase will always stick to ase.io.read function.

Copy link
Collaborator

@mjwen mjwen left a comment

Choose a reason for hiding this comment

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

Thanks @ipcamit. The updates look great. I just have a few minor comments. After this, I think we should be able to merge it.

kliff/dataset/dataset.py Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
kliff/dataset/dataset.py Outdated Show resolved Hide resolved
@mjwen mjwen merged commit 4dd7c6d into openkim:v1 Nov 14, 2023
3 checks passed
@mjwen mjwen added the feature label Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants