ACEsuit / ACEinterfaces

0 stars 3 forks source link

"free_energy" ASE calculator property #7

Open bernstei opened 2 years ago

bernstei commented 2 years ago

At least some operations of the testing framework (https://github.com/libAtoms/testing-framework) lead to an error with the free_energy property, e.g.

  File "/share/apps/ase/ase-local/2021-12-09/lib/python3.8/site-packages/ase/optimize/precon/lbfgs.py", line 365, in run
    return Optimizer.run(self, fmax, steps)
  File "/share/apps/ase/ase-local/2021-12-09/lib/python3.8/site-packages/ase/optimize/optimize.py", line 273, in run
    return Dynamics.run(self)
  File "/share/apps/ase/ase-local/2021-12-09/lib/python3.8/site-packages/ase/optimize/optimize.py", line 156, in run
    for converged in Dynamics.irun(self):
  File "/share/apps/ase/ase-local/2021-12-09/lib/python3.8/site-packages/ase/optimize/optimize.py", line 128, in irun
    self.log()
  File "/share/apps/ase/ase-local/2021-12-09/lib/python3.8/site-packages/ase/optimize/precon/lbfgs.py", line 381, in log
    e = self.atoms.get_potential_energy()
  File "/share/apps/ase/ase-local/2021-12-09/lib/python3.8/site-packages/ase/constraints.py", line 2469, in get_potential_energy
    atoms_energy = self.atoms.get_potential_energy(
  File "/share/apps/ase/ase-local/2021-12-09/lib/python3.8/site-packages/ase/atoms.py", line 728, in get_potential_energy
    energy = self._calc.get_potential_energy(
  File "/share/apps/ase/ase-local/2021-12-09/lib/python3.8/site-packages/ase/calculators/abc.py", line 24, in get_potential_energy
    return self.get_property(name, atoms)
  File "/share/apps/ase/ase-local/2021-12-09/lib/python3.8/site-packages/ase/calculators/calculator.py", line 484, in get_property
    raise PropertyNotImplementedError('{} property not implemented'
ase.calculators.calculator.PropertyNotImplementedError: free_energy property not implemented

I think potentials need to implement that property (which is really meant to be the version of the energy that's consistent with derivatives such as forces, originally for ASE DFT calculators) as being identical to the internal energy

bernstei commented 2 years ago

I think the issue happens with ExpCellFilter, which by default passes force_consistent=True to get_potential_energy(), and that triggers a calculate for the free_energy property.

cortner commented 2 years ago

Sorry, @davkovacs I'll need to leave this with you for now. But once you've identified exactly what we need at the Julia / JuLIP.jl / ACE.jl end I can help.

davkovacs commented 2 years ago

I see, so basically free energy should return the same as energy in our case? I saw it being implemented and returning simply the potential energy in other ASE calculators and never understood it. I will add it to the calculator.

davkovacs commented 2 years ago

@bernstei I believe it should be working now. Please let me know if it isn't, otherwise we can close this issue.

bernstei commented 2 years ago

Looks good now.

bernstei commented 2 years ago

I take this back. I think you need if 'energy' in properties or 'free_energy' in properties: in ACECalculator.calculate().

davkovacs commented 2 years ago

Can you please check if it works now on the latest commit, main branch?

bernstei commented 2 years ago

Yes, now it's identical to my locally patched version, which seems to work in every case I've tried so far.