Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adds a new section to README.md for containers #158

Merged
merged 3 commits into from
Jun 22, 2023

Conversation

SinaMostafanejad
Copy link
Member

Co-authored-by: Jonathan E. Moussa jemoussa@vt.edu

Summary

This PR adds a new section for the containerized version of MOPAC 22.0.6 alongside links to pertinent repositories, metadata and examples in the MolSSI Container Hub.

Status

  • Ready for merge

Co-authored-by:  Jonathan E. Moussa jemoussa@vt.edu
@SinaMostafanejad
Copy link
Member Author

SinaMostafanejad commented Jun 14, 2023

@godotalgorithm Please let me know if we you'd prefer to include a copy of the Dockerfile in the root directory of the MOPAC repository. Although the status is ready to merge, there might not be a dire need for this feature and we can simply keep this PR open as long as needed until we are almost certain we want this recipe as is.

@SinaMostafanejad
Copy link
Member Author

@godotalgorithm Let me know if there is any problem with the image(s), container(s) or example(s).

@codecov-commenter
Copy link

codecov-commenter commented Jun 15, 2023

Codecov Report

Merging #158 (20b3dda) into main (842293e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #158   +/-   ##
=======================================
  Coverage   66.68%   66.68%           
=======================================
  Files         331      331           
  Lines       73720    73720           
=======================================
  Hits        49157    49157           
  Misses      24563    24563           

@godotalgorithm
Copy link
Collaborator

Why are you specifying a version for MOPAC in conda? Wouldn't the Docker file/image be easier to maintain if no version was specified, which then defaults to installing whatever the latest version is?

@SinaMostafanejad
Copy link
Member Author

SinaMostafanejad commented Jun 21, 2023

Of course. We can remove that.

The reason behind pinning was twofold:

  • the lack of CI/CD infrastructure around our Dockerfile to catch pushes that cannot build healthy docker images which are normally pushed to a registry on Github, Dockerhub or elsewhere.
  • to ensure that MOPAC's dependent packages will not corrupt and/or are not incompatible with the existing (default) conda environment in the container as MOPAC's version changes. In such cases, one may normally get around the problem by adding options such as --no-deps or --freeze-installed at the installation time to shield the existing environment from such dependency updates.

@godotalgorithm
Copy link
Collaborator

The Dockerfile is installing the conda-forge version of MOPAC, which is only stable releases that have passed the testing & deployment process of conda. I think that is enough of a sanity check for the Docker images.

MOPAC's dependencies are quite minimal and hopefully stable. If MOPAC is having conda dependency problems, then a lot of other software is going to be having much bigger problems.

I'm fine with this as-is, and I'll merge it right after I finish merging a bug fix in a different PR.

@godotalgorithm godotalgorithm merged commit bcdfd89 into openmopac:main Jun 22, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants