ACEsuit / ACEpotentials.jl

Machine Learning Interatomic Potentials with the Atomic Cluster Expansion
MIT License
46 stars 12 forks source link

Review documentation #34

Closed gelzinyte closed 1 year ago

gelzinyte commented 2 years ago

I have added detailed documentation for ACE1pack to have something there, but still left the overall documentation a bit all over the place. Some suggested changes, discussion points & additions I gathered from various places & people:

  1. Review the conceptual & theory introduction - is all consistent, does it cover all what's needed, anything missing, ...?
  2. (updated) Hands-on docs: What should they focus on? My understanding is that ACE1pack is intended to be the default interface for ACE-users (not developers). In that case, I think the docs should focus more strongly on ACE1pack and have current in-julia ACE examples and explanations of ACE under "developer/similar" section, so there is less confusion about what is the intended/least fiddly way of fitting ACE1.
  3. (updated) ACE1pack-docs: currently, the docs show how to use ACE1pack in-julia. This is useful for detailed explanation, but actually, the least fiddlesome way of using ACE1pack would probably be to have script (in ACE1pack) that takes in a .json/.yaml parameter file and fits the potential. To-dos for that: a) move this script from workflow to here b) change docs to highlight fitting from command line & param file (including a mention that that's the way to do it from python).
  4. (done) Remove oudated/useless pages. E.g. datatypes seems niche and not-needed; solvers are covered in multiple places; ... ?
  5. Some short additions:
    • Add a short section on handling xyz's (probably in the "developer" section?)
    • Mention that using ACE1pack also imports IPFitting, ACE1, JuLIP exposing all of the functions therein. That said, julia will complain about using IPFitting, etc, because it is not formally "added".
    • Show how to evaluate basis functions (@casv2 example in #ace)

Any comments, anything else to add?

@cortner @gabor1 @WillBaldwin0 @casv2 @bernstei

cortner commented 2 years ago

Thank you for starting this discussion. I agree we should finally put together a decent draft of the docs.

2 - that's a very good point and I tentatively agree with you. To be discussed in the next meeting maybe?

3, 4 - yes, absolutely.

5 - this should be documented. If you don't formally add IPFitting, you can still import it via using ACE1pack.IPFitting a or IPFitting = ACE1pack.IPFitting

wcwitt commented 1 year ago

We are taking a close look at the documentation as part of the paper, so can we close this?

cortner commented 1 year ago

I think we've moved quite far beyond this.

@gelzinyte - if you disagree then please re-open.