ACEsuit / ACE.jl

Parameterisation of Equivariant Properties of Particle Systems
65 stars 15 forks source link

yace format cannot be read by python #102

Closed bernstei closed 2 years ago

bernstei commented 2 years ago

The default YAML reader (PyYAML?) cannot read yace format potentials as currently written, because they can include dictionaries that have lists as keys, while python only supports tuples as keys. Here's an example of a format that's produced, problem is on line 7.

This is really a big deal, since it's just meant to be read by the LAMMPS PACE package, but it would be convenient if a mutually understandable format could be used, so feel free to close it if it's not easy to solve.

deltaSplineBins: 0.001
E0:
  - -0.0046603
  - 0.0
  - 0.0
bonds:
  [0, 0]:
    rcut: 3.6
    xl: 0.37051039697542537
    recursion_coefficients:
      B:
        - 0.0
        - -5.913670422702299
        - -4.767753518631563
        - -4.334321380574182
        - -4.1097011882705266
        - -3.974709174650009
      A:
        - 31.295482159646728
        - 7.009373160773004
        - 5.651137307615811
        - 5.137397552378055
        - 4.871159051622799
        - 4.711155309520793
      C:
        - 0.0
        - 0.0
        - -0.8062257748298575
        - -0.9090909090909122
        - -0.9481763873555094
        - -0.9671528397232746
    pl: 2
    radbasename: "ACE.jl.base"
    r0: 1.8
    xr: 1.3168503090567045
    maxn: 6
    pr: 2
    p: 2
casv2 commented 2 years ago

This is really a big deal

Is or isn't? As you say the exported potentials are really only designed to be used with the PACE calculator.

The PACE potentials are read using the C YAML library and used as a PACE evaluator (written by the Drautz group). Our exported ACE potentials should be identical in structure to Drautz' ACE potentials, and both theirs and ours should be read as PACE/LAMMPS evaluators without any problems. I don't think anyone has ever tried importing them inside Python.

That being said, the PACE export function is a big hack indeed... We've had to reformulate the ACE basis in their language which was a bit of a pain and feels hacky...

bernstei commented 2 years ago

Sorry, typo - supposed to be "isn't".

bernstei commented 2 years ago

Anyway, I was just trying to use yaml.safe_load("ace_pot.yace") as a way of testing the validity of the file created as part of my ACE fitting wfl wrapper, but that's hardly essential. It's pretty hard to come up with a situation where the yace file is written but does not contain valid content.

casv2 commented 2 years ago

Perhaps easiest would be to perform a unit test? Evaluate the potential on a few isolated atoms and see whether the E0's agree.

Or maybe just look for the E0's by reading the .yaml as text and check whether they match the provided E0's by the user... Not sure this is really useful though

bernstei commented 2 years ago

It's not a big deal - it was just meant as a sanity check, mostly for situations where e.g. the script got killed while writing.

I guess if it's possible to create an ASE Calculator from a yace I could do that and make sure it doesn't fail. Right now for json output I just parse it, not actually try to create a Calculator.

cortner commented 2 years ago

It’s not an immediate problem but so think it is a problem. Does this concern Only OUR ace files or Ralf’s group’s files as well?

casv2 commented 2 years ago

Does this concern Only OUR ace files or Ralf’s group’s files as well?

Both, Yury has shaped the ACE .yamls this way and both our files are read by the C YAML library without problems. Apparently trying to read them into Python causes problems...? Weird I know, but I think it's because as mentioned previously:

python only supports tuples as keys

This is not a problem for the PACE / LAMMPs evaluator since it doesn't use Python.

cortner commented 2 years ago

Yes but if we want an ecosystem then that’s bad.

bernstei commented 2 years ago

Note that there's currently no python code that does anything useful with yace files, as far as I know. It would only be useful as a minimal check on the format of the file being valid yaml. Supposedly ruamel's python YAML reader can supposedly read such yaml files, and automatically convert the lists to tuples, or somethingng.

Out of curiosity, can you create a Julia ACE (never mind an ASE Calculator) from a yace file?

casv2 commented 2 years ago

No we currently don't have an option to create a Julia ACE from a .yace file. I'm not sure it would be used much actually, but from an ecosystem perspective it would be nice to have indeed...

bernstei commented 2 years ago

I'm not sure it would be used much actually,

I agree - it was just a thought, as an alternative for my consistency check use-case.

cortner commented 2 years ago

Hm ... let's keep in mind that YACE is really not our format. In fact I don't like it anyway and don't think it is particularly useful for supporting the eco-system. So I will take the point and keep this in mind when we continue discussions with Ralf's group about sharing codes and models, but I just don't think this is an ACE.jl issue. We should think of YACE simply as an export to the PACE code and nothing else.

Are you both happy with that response and happy to close this issue?

bernstei commented 2 years ago

On Jan 11, 2022, at 6:27 PM, Christoph Ortner @.***> wrote:

Hm ... let's keep in mind that YACE is really not our format. In fact I don't like it anyway and don't think it is particularly useful for supporting the eco-system. So I will take the point and keep this in mind when we continue discussions with Ralf's group about sharing codes and models, but I just don't think this is an ACE.jl issue. We should think of YACE simply as an export to the PACE code and nothing else.

Are you both happy with that response and happy to close this issue?

I'm certainly happy for you not to support yace beyond writing some version of it that works with lammps (although I think it is really important to support LAMMPS, and preferably without explicit involvement of julia at runtime).

cortner commented 2 years ago

That will be incredibly difficult to achieve long-term. We are very well aware of the issue, but Julia has such a rich type system that will be difficult to reproduce in a statically compiled code and the more complex the models become the more difficult it will be to support Lammps with a simple static library that doesn't call Julia.

cortner commented 2 years ago

but this is a much longer and much more difficult discussion for another day.