search

MDAnalysis / mdanalysis

MDAnalysis is a Python library to analyze molecular dynamics simulations.
https://mdanalysis.org
Other
1.31k stars 650 forks source link

Future of chemfiles reader in MDA #2731

Open Luthaf opened 4 years ago

Luthaf commented 4 years ago

In #1862, I added coordinate & topology readers and writers based on chemfiles, which are disabled by default. I am opening this issue to discuss the future of this code, and how to make it available to as many people as possible.

A possible starting point would be enabling chemfiles as a reader for formats not yet supported by MDA, which currently are:

Would you be interested in activating chemfiles by default for some/all of these formats?

Another area where chemfiles can help with MDA is in formats which both implement, where chemfiles might be a bit faster (thanks to @richardjgowers tricks of faster line by line reading). This includes formats like PDB, XYZ, Tinker XYZ and potentially others.

Here I would leave it to the users to pick which parser they want, but potentially add some documentations so that the existing alternative is more visible to end users.

Related to the previous point, chemfiles is able to parse some files (such as non standard PDB files) not supported by MDA (#988, #1133, #1966 are examples of issues with the PDB parser which should work with chemfiles). Since chemfiles also has bugs (just different ones), I would take the same action as the previous point, telling users there is an alternative parser that they can use if the default one do not works.

This alternative parser suggestion could also work with other bridges to MDA such as the RDKit one from #2707

What do you think?

orbeckst commented 4 years ago

Sorry for the lack of discussion at the moment. I think at the moment many @MDAnalysis/coredevs are involved in GSoC and finalizing release 1.0 #2443 / fixing issues or preparing for 2.0 #2729 . Thank you for your patience.

I added coordinate & topology readers and writers based on chemfiles, which are disabled by default.

Can you explain what you mean by "disabled by default"? Do you mean there's not automatic detection of the file format and rerouting to ChemfilesReader?

As a sidenote, I just wanted to mention that interoperability (at the API level) has become a new theme in MDAnalysis, with the idea to use converters. I think in the longer run we should also consider making sure that the chemfiles python bindings can function within the converter framework as well.

Luthaf commented 4 years ago

Yes, by disabled by default I meant no automating routine to ChemfilesReader. As far as I understand, users need to request the use of ChemfilesReader explicitly.

Thanks for the pointer on converters, I'll read the documentation on this subject!