add trvdos - #391
Conversation
There was a problem hiding this comment.
Thank you for submitting trvdos as an MDAKit. I have a number of comments that if addressed would improve the kit.
There appears to be an issue with the registry in that the CI did not run. I am looking into that. The CI is needed for the initial review.
UPDATE: With the CI running again, the main issue is the lack of installation commands for the gsl library (see additional comments inline)
| project_home: https://github.com/HeydenLabASU-collab/trvdos | ||
|
|
||
| ## str: the link to the project's documentation | ||
| documentation_home: https://trvdos.readthedocs.io/en/latest/ |
There was a problem hiding this comment.
The docs (online and README) should contain a short explanation what problem the code solves and the key algorithmic ideas. A minimal usage example should also be included.
Minimal documentation: what your code does, how to install it, and how to use it
See section on docs in the MDAKit paper of what's ideal:
Describing what a given code does and how to use it is a key component of open sharing. Ideally a package would include a complete description of the entire codebase, including both API documentation and some kind of user guide with worked examples on how the code could be used in certain scenarios. Whilst this is recommended as best practices for an MDAKit, we recognize that this is not always feasible, especially in the early stages of development. Therefore, the minimum requirement for MDAKits is to have a readme file which details the key aspects of the MDAKit, such as what it is intended to do, how to install it, and a basic usage example.
There was a problem hiding this comment.
Some of the badges in the README are not working properly, possibly related to HeydenLabASU-collab/trvdos#6
|
@IAlibay do you know why the registry CI workflow did not run on this PR? |
|
I'll try cycling CI by closing/reopening. |
|
That started the the proper CI runners. Not sure why it didn't do it the first time. Maybe for first-time committers?? |
No that won't be it since it would be blocked on re-open. This is likely just github having a hard time, it's becoming incresingly frequent nowadays. |
orbeckst
left a comment
orbeckst
left a comment
There was a problem hiding this comment.
Great that you fixed the installation. The only remaining issue are the docs improvements. Please ping me when you need me to review again.
3 participants
Adds trvdos, an analysis module for calculating velocity autocorrelation functions (VACF) and velocity density of states (VDoS) from MD trajectories.