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

ASE calculator refactor and jit dipole models #95

Merged
merged 17 commits into from
May 24, 2023

Conversation

davkovacs
Copy link
Collaborator

No description provided.

@ilyes319 ilyes319 self-requested a review March 31, 2023 14:09
@ilyes319
Copy link
Contributor

We should add the variance and mean to the properties for writing issues.

@davkovacs
Copy link
Collaborator Author

davkovacs commented Apr 26, 2023

We should add the variance and mean to the properties for writing issues.

Done in the above commit, I think it is ready to merge (if you verify that there is not performance difference to the previous calculator. )

@ilyes319
Copy link
Contributor

ilyes319 commented May 1, 2023

Better to compute the mean and variance of the forces on the GPU and then transfer that one time to cpu.

@bernstei
Copy link
Collaborator

bernstei commented May 1, 2023 via email

@ilyes319
Copy link
Contributor

ilyes319 commented May 1, 2023

That's a good point. Maybe add a flag to enable it.

@davkovacs
Copy link
Collaborator Author

After discussing with Cas we agreed that it is sufficient to return the mean and variance of energies and forces, it shouldn't be necessary to return the output force for all models in a committee for HAL.

@gabor1
Copy link
Collaborator

gabor1 commented May 9, 2023 via email

@bernstei
Copy link
Collaborator

bernstei commented May 9, 2023

After discussing with Cas we agreed that it is sufficient to return the mean and variance of energies and forces, it shouldn't be necessary to return the output force for all models in a committee for HAL.

Are you sure? This is my bias force expression from ACEHAL. Can it really be refactored into something that doesn't depend on each energy and force separately?

            F_bias = np.mean([(E_comm_member - E) * (F_comm_member - F) for E_comm_member, F_comm_member in zip(Es, Fs)], axis=0)
            F_bias /= E_bias

@davkovacs
Copy link
Collaborator Author

Thanks @bernstei I clarified again with Cas and have changed to return all committee forces.

@ilyes319 ilyes319 merged commit f6d48af into ACEsuit:develop May 24, 2023
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.

4 participants