MDIL-SNU / SevenNet

SevenNet - a graph neural network interatomic potential package supporting efficient multi-GPU parallel molecular dynamics simulations.
https://pubs.acs.org/doi/10.1021/acs.jctc.4c00190
GNU General Public License v3.0
133 stars 17 forks source link

[Feat] change ase.neighborlist to from matscipy.neighbours #124

Open hexagonrose opened 2 weeks ago

YutackPark commented 2 weeks ago

Thanks for the contribution. Some questions:

  1. Why we use this for only unlabeled_** routine? It will make preprocessing faster if implemented to atoms_to_graph function.

  2. It seems doing some hacky things if the given system is non-periodic. Is this tested for non-periodic systems?

hexagonrose commented 2 weeks ago
  1. Oh, I missed the atoms_to_graph function, which also uses ase.neighborlist. In sevennet_calculator.py, it only uses unlabeled_atoms_to_graph, so I only fixed that part.

  2. MACE also relies on those hacky things. I haven't yet tested it further, like MD simulations, but without these hacky things, it triggered a singular matrix error (molecules and isolated atom).

YutackPark commented 2 weeks ago

It's fine about atoms_to_graph, I just wondered if there exists some reason preventing implementaion. It is also absolutely fine to use hacky things, but if it is related to specific cases (non-pbc), it should be tested before merge. If my memory is correct, there already exists non-pbc calculator test. But double check is always a good idea. I'll check the first case (atoms_to_grpah). Thanks again for the contribution!

2024년 11월 7일 (목) 오후 10:31, hexagonrose @.***>님이 작성:

1.

Oh, I missed the atoms_to_graph function, which also uses ase.neighborlist. In sevennet_calculator.py, it only uses unlabeled_atoms_to_graph, so I only fixed that part. 2.

MACE also relies on those hacky things. I haven't yet tested it further, like MD simulations, but without these hacky things, it triggered a singular matrix error (molecules and isolated atom).

— Reply to this email directly, view it on GitHub https://github.com/MDIL-SNU/SevenNet/pull/124#issuecomment-2462249796, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2RQY2Z2ALY34WO55YZQEQDZ7NTTZAVCNFSM6AAAAABRKVP6VGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRSGI2DSNZZGY . You are receiving this because you commented.Message ID: @.***>