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

schedulers: use num_cores_per_mpiproc in direct #5051

Conversation

dev-zero
Copy link
Contributor

@dev-zero dev-zero commented Aug 4, 2021

No description provided.

@dev-zero dev-zero force-pushed the feature/recognize-cores_per_mpiproc-direct-sched branch from c97745e to 87faf08 Compare August 4, 2021 14:11
@dev-zero dev-zero requested a review from sphuber August 4, 2021 14:24
@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #5051 (87faf08) into develop (78ef633) will decrease coverage by 0.01%.
The diff coverage is 37.50%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5051      +/-   ##
===========================================
- Coverage    80.23%   80.23%   -0.00%     
===========================================
  Files          515      515              
  Lines        36757    36762       +5     
===========================================
+ Hits         29490    29493       +3     
- Misses        7267     7269       +2     
Flag Coverage Δ
django 74.72% <37.50%> (-<0.01%) ⬇️
sqlalchemy 73.62% <37.50%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
aiida/schedulers/plugins/direct.py 61.18% <37.50%> (-0.03%) ⬇️

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 78ef633...87faf08. Read the comment docs.

@giovannipizzi
Copy link
Member

Thanks @dev-zero !

Can you please add one test along the lines of, e.g., this one, in the appropriate file?

If @sphuber has no further comment, then for me it can be merged
(I guess it can be useful also on e.g. Quantum Mobile, related to the issues of the number of OpenMP threads, so pinging @chrisjsewell for info)

@sphuber
Copy link
Contributor

sphuber commented Aug 12, 2021

Looks fine to me as well, but I must say that I am not too experienced with this stuff. Is there any environment thinkable where exporting OMP_NUM_THREADS can have an effect that one wouldn't want? Or is it that at worst it does nothing? If so, then it is fine for me to add, after a small test is added.

@sphuber
Copy link
Contributor

sphuber commented Sep 9, 2021

I have added tests, but I don't have writing rights to @dev-zero 's fork. To get this merged I pushed this to my fork and created a new PR #5126 Attribution is kept of course. Hope you don't mind

@sphuber sphuber closed this Sep 9, 2021
@dev-zero dev-zero deleted the feature/recognize-cores_per_mpiproc-direct-sched branch September 14, 2021 09:41
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.

3 participants