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

Colabfit enhancments by Eric #162

Merged
merged 4 commits into from
Feb 10, 2024
Merged

Colabfit enhancments by Eric #162

merged 4 commits into from
Feb 10, 2024

Conversation

ipcamit
Copy link

@ipcamit ipcamit commented Feb 8, 2024

This PR consists changes made by Eric to colabfit io. List of changes:

Edited functions:

  1. Dataset.from_colabfit
  2. Dataset.add_from_colabfit
  3. Dataset._read_from_coabfit
  4. Configuration.from_colabfit

Added functions:

  1. Configuration.to_ase_atoms: converts KLIFF configuration to ASE atoms configuration. Used for storing the KLIFF dataset back to Colabfit.

Other changes:
using from chemical_symbols ase.data

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.

the order of ase_atoms_list, and path should be switched in

instance.add_from_ase(
            ase_atoms_list, path, weight, energy_key, forces_key, slices, file_format
        )
        return instance

as well.

It is called in from_ase method. See #161

@mjwen
Copy link
Collaborator

mjwen commented Feb 9, 2024

Are we planning to add tests for this class, so that we can avoid such bugs?

@mjwen mjwen added the fix label Feb 9, 2024
@ipcamit
Copy link
Author

ipcamit commented Feb 9, 2024

Yes. Next PR would be of tests for this and transforms modules. It was on hold has I had to fix the descriptor library first. Finished it this week, so KLIFF tests are next.

@mjwen mjwen merged commit 6741f52 into openkim:v1 Feb 10, 2024
4 checks passed
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.

None yet

2 participants