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

Docker container for ci test #180

Merged
merged 8 commits into from
May 3, 2023
Merged

Docker container for ci test #180

merged 8 commits into from
May 3, 2023

Conversation

dreamer2368
Copy link
Collaborator

@dreamer2368 dreamer2368 commented Feb 5, 2023

By using docker container, github ci test can contain only libROM-related parts for testing. Most installation steps for dependencies can be skipped, which can reduce the linux ci test time from about 15 mins to 5 mins.

  • The recipe for the docker container is stored as .github/workflows/Dockerfile.
  • The following steps are deleted, because all dependencies including googletest are pre-installed in the container:
    • Install Linux Dependencies
    • ./.github/workflows/checkout_repo
    • run: scripts/setup.sh
  • Set Swap Space step is not needed any longer, because all dependencies are not installed within ci test. However, this step is still included in the CI test, just in case we may need extra space in the future.

Note:

  • The docker container keeps the specific version (or commit) of mfem. While it necessitates manually updating the container occasionally, it also enables a stable CI test and development.

Issue:

  • Mac CI test has been failing before this pull request, and this issue is separately handled by the pull request Fix libROM CI Failure on MacOS 12 #175. Removed from the ci workflow .
  • RandomizedSVDTest is susceptible to round-off error, failing randomly on different machine. This affects the test result under the container. See Issue test_RandomizedSVD failure #193 . Commented the corresponding test until the issue is resolved by a separate pull request.

TODO:

@dreamer2368 dreamer2368 force-pushed the docker_ci branch 12 times, most recently from d35c6b5 to 3fed7e0 Compare February 5, 2023 05:06
@dreamer2368 dreamer2368 changed the title Suggestion of using docker container for ci test Docker container for ci test Feb 5, 2023
@dreamer2368 dreamer2368 marked this pull request as ready for review March 11, 2023 10:11
@chldkdtn chldkdtn requested a review from debog May 3, 2023 04:02
@chldkdtn
Copy link
Collaborator

chldkdtn commented May 3, 2023

@debog @dylan-copeland @siuwuncheung @sullan2 Would you mind reviewing this PR?

@chldkdtn chldkdtn requested a review from sullan2 May 3, 2023 04:04
@debog
Copy link
Collaborator

debog commented May 3, 2023

@debog @dylan-copeland @siuwuncheung @sullan2 Would you mind reviewing this PR?

I think I am out of my depths here!

@dylan-copeland
Copy link
Collaborator

@debog @dylan-copeland @siuwuncheung @sullan2 Would you mind reviewing this PR?

I think I am out of my depths here!

Me too, and there are enough approvals already, so I will not review.

@dreamer2368 dreamer2368 merged commit c0da369 into master May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants