Mapped ase md and Flatten node #299
Conversation
for more information, see https://pre-commit.ci
…nto mapped-ase-md
|
So far the implementation is ready and tested. Will get an overhaul as soon as it is possible to safe more than one trajectory in h5 in a structured way. |
There was a problem hiding this comment.
I think this will be a great addition but I have some comments.
Something I'm undecided is the split into data_id and data_ids parameters.
We could easily unify them into data_ids: list[int] and if run we mandate that len(data_ids) == 1 ? But on the other hand, using different names makes it more clear what is goind on.
ipsuite/calculators/ase_md.py
Outdated
| # File can not be opened with DVCFileSystem, try normal open | ||
| atoms = list(ase.io.iread(self.data_file)) |
There was a problem hiding this comment.
This is dangerous, where is this required?
There was a problem hiding this comment.
That is a good point. Actually I modified the ProcessAtoms base class which also uses that line, and have to admit that I don't know if it is needed. Maybe we can talk about the atoms and file handling so I can adjust this in the base class too?
There was a problem hiding this comment.
Should be address this in a separate PR?
…nto mapped-ase-md
for more information, see https://pre-commit.ci
…nto mapped-ase-md
for more information, see https://pre-commit.ci
|
Also solves issue #301 |
No description provided.