Fixing the estimation of available cpus for a machine (localhost) #971
Conversation
(now under the `machine_cpus` variable, no more `localhost_cpus`) This is done by using the `get_default_mpiprocs_per_machine` method of the computer node.
for more information, see https://pre-commit.ci
I take the information from the computer setup: ```python default_mpiprocs = computer.get_default_mpiprocs_per_machine() ```
for more information, see https://pre-commit.ci
mikibonacci
changed the title
| self.num_cpus.value = 1 | ||
| self.num_cpus.description = "CPUs" | ||
| else: | ||
| default_mpiprocs = computer.get_default_mpiprocs_per_machine() | ||
| self.num_nodes.disabled = False | ||
| self.num_cpus.max = default_mpiprocs | ||
| self.num_cpus.value = default_mpiprocs | ||
| self.num_cpus.value = ( |
There was a problem hiding this comment.
this can fix the number of max cpus we can choose for a code, avoid the erroneous os.cpu_count
There was a problem hiding this comment.
Why setting num_cpus to 1 if it's localhost?
There was a problem hiding this comment.
I guess (was already 1, before this PR) that the reason is not to use, by default, the whole resources of the host, if localhost. Imagine in the demo server, you will be then always suggest as default to use all the resources, maybe is not the ideal case. But I am open to change it
There was a problem hiding this comment.
Thanks for the explanation. It sounds reasonable to use 1.
| localhost_cpus = os.cpu_count() | ||
| machine_cpus = orm.load_node( | ||
| pw_code_model.selected | ||
| ).computer.get_default_mpiprocs_per_machine() |
There was a problem hiding this comment.
I called machine_cpus as is not more only for localhost_cpus
There was a problem hiding this comment.
Hi @mikibonacci , thanks for the work. I am not quite understand the fix. Maybe @unkcpz know better on this part, please have a look.
There was a problem hiding this comment.
Thanks @mikibonacci, for getting the set default CPU number get_default_mpiprocs_per_machine is the correct way I was putting for the computer. So this part looks good to me.
Co-authored-by: Jusong Yu <[email protected]>
for more information, see https://pre-commit.ci
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #971 +/- ##
==========================================
+ Coverage 68.40% 68.42% +0.01%
==========================================
Files 111 111
Lines 6293 6289 -4
==========================================
- Hits 4305 4303 -2
+ Misses 1988 1986 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This fixes #937
This fixes #888
Issue #937 is fixed here,
while #888 is fixed here