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

Force unit in datasets #298

Open
wiederm opened this issue Oct 25, 2024 · 2 comments
Open

Force unit in datasets #298

wiederm opened this issue Oct 25, 2024 · 2 comments

Comments

@wiederm
Copy link
Member

wiederm commented Oct 25, 2024

I might misunderstand the curation pipeline, but it seems to me that energies and positions are correctly in kJ/mol and nanometer, but the force in some of the datasets is in kJ/mol/angstrom.

@chrisiacovella , can you clarify if I am missing something here?

@chrisiacovella
Copy link
Member

This seems to have been a mistake (or an accidental hold over when we actually were using angstroms internally).

I think this is a 2 stage fix:

  1. Add unit conversion into the data loader. We have discussed this in the past as an extra measure to ensure all units are consistent, but we had not prioritized it. this will allow us to use existing datasets without our target forces being an order of magnitude smaller.
  2. Change units in the datasets. This is easy, but uploading to zenodo is a bit time consuming. As discussed in a separate issue Revamp the datasets/dataset class #267, we can simplify our additional tags in the dataset (that tell us how to parse the numpy arrays) to only being per_system, per_atom, and metadata. changing units, these optional tags, and associated loader, can all be done in the same PR.

I will do a PR for stage 1 now.

@chrisiacovella
Copy link
Member

#1 has been address in PR #301. As mentioned in the PR, it would be good to now include a test to ensure proper unit conversion. This will require creating a new test dataset, so I'll do that in a separate PR.

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

No branches or pull requests

2 participants