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

Discussion of ZeroVirialCalculator #11

Closed
cortner opened this issue May 25, 2024 · 3 comments
Closed

Discussion of ZeroVirialCalculator #11

cortner opened this issue May 25, 2024 · 3 comments

Comments

@cortner
Copy link
Member

cortner commented May 25, 2024

I'm not sure what the actual intention is. If the intention is what you say in the documentation then that is incredibly dangerous. If a computation gets a zero-virial for a calculator that just doesn't have virial implemented then that will silently give incorrect results.

The correct way - in my view - to implement this is to return missing.

@tjjarvinen
Copy link
Collaborator

Basicly, if calculator does not support virial, there should not be available virial call to inform used it does not exist.

Second thing i-PI interface requires virial to be returned, even if it is not used.

So, you could say that ZeroVirialCalcualtor is implemented as a helper for i-PI calculator. The easiest way to use i-PI calculator for constant volume simulation is to wrap your calculator into ZeroVirialCalculator. That will then allow easy use of calculators that do not support virial. Also, you do that by knowing you cheat with virials is better than an implementation that does that under the hood.

@cortner
Copy link
Member Author

cortner commented May 25, 2024

I don't have a problem with the i-PI thing. But that is not what the documentation says right now. It makes sense in a language that requires specific type a priori. In Julia this does not make sense.

I don't have a problem providing that utility calculation, but the documentation needs to be fixed. It could even be given the name UnsafeZeroVirials to highlight that this should not normally be done unless you really know exactly what you are doing.

@cortner
Copy link
Member Author

cortner commented Aug 10, 2024

based on the notes you added should this be closed?

@cortner cortner closed this as completed Aug 11, 2024
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