search

snurr-group / mofid

A system for rapid identification and analysis of metal-organic frameworks
https://snurr-group.github.io/mofid/
GNU General Public License v2.0
42 stars 24 forks source link

Containerize mofid package #28

Closed bbucior closed 2 years ago

bbucior commented 2 years ago

Hello @arosen93 I finished the Docker and singularity experiments! Our setup requirements are now much shorter (though there might be cluster-specific instructions users need to follow to get set up with singularity on their HPC environment).

The link to the .sif file in the README points to the future location for a tagged GitHub release. In the meantime, I've published a pre-release file (and will delete and rerelease once we finalize the PR).

I've only been able to test out the code on my laptop so far. Would you be able to test on HPC? Any other feedback would be appreciated, too! Thanks for your help.

Andrew-S-Rosen commented 2 years ago

Beautiful!! I'll try it out tonight and respond more here. Stay tuned.

Andrew-S-Rosen commented 2 years ago

One general suggestion before I begin. I would recommend that we not de-emphasize the existing installation and usage instructions. I know personally, I would always prefer to run things without Docker so would be a bit scared off by the instruction as-written. People might be a bit confused too if they are coming back to use MOFid after some time away. I would propose that we keep the README basically as it currently is but add a new section for containers. What are your thoughts about this?

If the concern is that it might get too bulky for the README (which I think it would be), maybe the better solution is to have no installation instructions in the README at all and simply have two hyperlinks: one to a .md on the existing instructions and one for Docker.

Edit: I'm editing the README now to propose some changes.

Andrew-S-Rosen commented 2 years ago

I was able to get everything to run on HPC w/ Singularity without any issue!

Here is a suggestion for an updated README: https://github.com/arosen93/mofid/blob/rosen-docker/README.md. I can open the PR after yours is merged.

bbucior commented 2 years ago

Great point about user preferences. I really like your updated READMEs and how clean all of the setup and usage instructions are presented--thanks for simplifying those docs! I'll issue a new release and SIF after this PR and yours are merged.

Andrew-S-Rosen commented 2 years ago

Great! I was pleasantly surprised by how easy the singularity container was to use. Thanks for putting that together! I'll merge the README PR when the test passes.