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: Default to number of physical cores for localhost code. #5161

Merged
merged 3 commits into from
Oct 5, 2021

Conversation

csadorf
Copy link
Contributor

@csadorf csadorf commented Oct 5, 2021

Adjust how the "localhost" code is configured on the aiida-core Docker
image. The default value is based on the actual number of physical cores
and is further configurable via an environment variable.

Closes #5117.

Adjust how the "localhost" code is configured on the aiida-core Docker
image. The default value is based on the actual number of physical cores
and is further configurable via an environment variable.

Closes #5117.
@csadorf csadorf enabled auto-merge (squash) October 5, 2021 09:18
@csadorf csadorf requested review from sphuber and yakutovicha October 5, 2021 09:39
@csadorf csadorf disabled auto-merge October 5, 2021 09:39
@codecov
Copy link

codecov bot commented Oct 5, 2021

Codecov Report

Merging #5161 (2ec3d33) into develop (b1773f8) will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5161      +/-   ##
===========================================
- Coverage    80.93%   80.91%   -0.02%     
===========================================
  Files          536      536              
  Lines        37056    37056              
===========================================
- Hits         29989    29979      -10     
- Misses        7067     7077      +10     
Flag Coverage Δ
django 75.76% <ø> (-0.02%) ⬇️
sqlalchemy 74.91% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aiida/engine/processes/calcjobs/manager.py 83.19% <0.00%> (-3.53%) ⬇️
aiida/engine/daemon/client.py 75.26% <0.00%> (-1.01%) ⬇️
aiida/engine/processes/calcjobs/tasks.py 64.77% <0.00%> (-1.00%) ⬇️
aiida/orm/nodes/process/calculation/calcjob.py 77.04% <0.00%> (-0.95%) ⬇️
aiida/transports/plugins/local.py 81.66% <0.00%> (+0.26%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1773f8...2ec3d33. Read the comment docs.

Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @csadorf , some comments below

.docker/opt/configure-aiida.sh Outdated Show resolved Hide resolved
.docker/opt/configure-aiida.sh Outdated Show resolved Hide resolved
And allow for explicit user override even in the case of success. The
number of logical cores servers only as the default value.
@csadorf csadorf requested a review from sphuber October 5, 2021 12:28
Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @csadorf . All good, I just think you forgot to replace one occurrence of MPI_PROCS_PER_MACHINE. Once corrected this can be merged.

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
@csadorf
Copy link
Contributor Author

csadorf commented Oct 5, 2021

Thanks @csadorf . All good, I just think you forgot to replace one occurrence of MPI_PROCS_PER_MACHINE. Once corrected this can be merged.

Looks like a git conflict resolution went wrong...

@csadorf csadorf requested a review from sphuber October 5, 2021 13:56
@csadorf
Copy link
Contributor Author

csadorf commented Oct 5, 2021

No idea why the tests are not passing anymore.

@sphuber
Copy link
Contributor

sphuber commented Oct 5, 2021

No idea why the tests are not passing anymore.

They were not even running because Github Actions is in degraded mode: https://www.githubstatus.com/

@sphuber sphuber merged commit 5480267 into develop Oct 5, 2021
@sphuber sphuber deleted the fix/issue-5117 branch October 5, 2021 19:08
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.

The localhost computer is hard-coded to have only one MPI core
2 participants