search

ACEsuit / ACEfit.jl

Generic Codes for Fitting ACE models
MIT License
7 stars 6 forks source link

What happened to `Dat`? #39

Open cortner opened 1 year ago

cortner commented 1 year ago

@wcwitt -- I'm currently trying to implement a fitting script for a new project and noticed for the first time how much the structure of ACEfit has changed. The new AtomsData is now very restrictive and moreover seems to require far more code overhead that the old code that was inspired by IPFitting. I'm guess there were good reasons for those changes though but I don't remember the discussion. Can you remind me please?

Depending on this, I may bring the old datastructures back. As far as I can tell they can easily live side-by-side with your new framework.

cortner commented 1 year ago

... so my first impression is that the current model is very general and would be very suitable for a package such as ACE1.jl + ACE1pack.jl where it is worthwhile writing out in detail what kind of observations we like. But it is not flexible. If I want to introduce a new kind of observation I cannot do this easily e.g. by just defining a new local observation type or even monkey-patching ACE1pack.jl. I am missing that possibility.

wcwitt commented 1 year ago

There were several reasons for moving things around ... I'm trying to remember.

One was this request: https://github.com/ACEsuit/ACEfit.jl/issues/31. That led to me defining a very general AbstractData in ACEfit, which is fleshed out in ACE1pack. (And could be fleshed out in other ways for other fitting purposes.)

I think the restructuring was also better for distributed fitting somehow, but don't recall exactly.

wcwitt commented 1 year ago

But I see your point. It's probably worth having a very simple Dat in ACEfit that is still problem agnostic somehow.

wcwitt commented 1 year ago

And I can restore some of the ObsEnergy, ObsForce, and ObsVirial stuff if you prefer ... if I remember correctly I found that part of IPFitting challenging to parse, so my compromise was to make a very abstract AbstractData in ACEfit and a very concrete AtomsData in ACE1pack.

cortner commented 1 year ago

Maybe the best way forward is if I first try to make the ACE1pack datastructure as flexible as I would like it to be and then we can see how much of that can be moved back into ACEfit.

cortner commented 1 year ago

as a hook: immagine you want to add new observations about charges or magnetic moments.