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

What happened to Dat? #39

Open
cortner opened this issue Jan 9, 2023 · 6 comments
Open

What happened to Dat? #39

cortner opened this issue Jan 9, 2023 · 6 comments

Comments

@cortner
Copy link
Member

cortner commented Jan 9, 2023

@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
Copy link
Member Author

cortner commented Jan 9, 2023

... 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
Copy link
Collaborator

wcwitt commented Jan 9, 2023

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

One was this request: #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
Copy link
Collaborator

wcwitt commented Jan 9, 2023

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

@wcwitt
Copy link
Collaborator

wcwitt commented Jan 9, 2023

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
Copy link
Member Author

cortner commented Jan 9, 2023

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
Copy link
Member Author

cortner commented Jan 9, 2023

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

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