search

ACEsuit / ACE.jl

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

Move Bonds out of ACE.jl #121

Closed cortner closed 2 years ago

cortner commented 2 years ago

@zhanglw0521 @MatthiasSachs How do you feel about moving bonds out of ACE.jl into a new repo ACEbonds.jl. That way I can keep moving ACE.jl along without maintaining compatibility with the bonds implementation.

zhanglw0521 commented 2 years ago

I am ok with that. Just one thing to clarify - will the cylindrical/ellipsoidal cutoff also be moved to ACEbonds.jl or do they still lie in ACE.jl?

cortner commented 2 years ago

I want to move anything related to bonds into ACEbonds.jl. ACE.jl is getting too complex for me to keep it all in my head. And this is a well defined group of functionalities that can be removed - or, rather, separated out.

zhanglw0521 commented 2 years ago

That was how I understood the proposal too. I just think that those two cutoffs can go with ACEbonds.jl as I can hardly come up with other applications of these kinds of cutoff. If there is any, it must be really similar to bonds and we can then consider renaming ACEbonds.jl to something else.

cortner commented 2 years ago

ok we want the same thing and I agree with you that this is the main / maybe only application.

cortner commented 2 years ago

I would actually consider moving Bonds back into ACE.jl but only much later once it has fully stabilized and we are starting to enforce backward compatibility.

MatthiasSachs commented 2 years ago

Yes, I don't mind at all! Who is going to set up the ACEbonds.jl repository?

cortner commented 2 years ago

I will set it up, but I won't mind at all if the two of you want to coordinate to get a first version to work.

cortner commented 2 years ago

There is NO RUSH whatsoever for this I think.

cortner commented 2 years ago

Thank you for agreeing. I'll go ahead and remove all bonds code after the current PR I'm working on.

cortner commented 2 years ago

done - cf #126

The last version of ACE.jl that comes with bonds included is version = "0.12.33" - please make this an upper bound in your projects. Once the new ACEbonds is ready you can then move over your projects to that package.

cortner commented 2 years ago

0.12.34 is now tagged as the first version without the bonds code. So ACEbonds.jl needs to use 0.12.34 as a lower bound for ACE.jl