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

Fix uc calculation and add warning #84

Merged
merged 2 commits into from
Jan 17, 2024

Conversation

skrakau
Copy link
Collaborator

@skrakau skrakau commented Jan 11, 2024

I got an error for a case where the reported cpu_usage was 0.0.

In this context I realised that I did not use the actual number of requested CPUs nc to compute uc, but a rounded value obtained from the nextflow cpu_usage metric. Not sure why exactly I did that.

This caused an error when cpu_usage was 0.0, but it is also bad, since for the final formula the nc value was used, and this would cause erroneous results if those nc and cpus_ceil would differ.

I changed it to only use nc now. Additionally I added a warning if cpu_usage is 0.0, since this might indicate that something is wrong as well.

@mirpedrol could you double check?

@skrakau skrakau marked this pull request as ready for review January 12, 2024 11:45
@mirpedrol
Copy link
Collaborator

I think it makes sense to use the number of cpus to calculate uc. nc and the value that we were calculating (cpu_ceil) should be the same value most of the times. But now I checked how to find cpu usage, according to the Green algorithms FAQ (How do I find the usage factor of my processors?) there are some commands such as seff for SLURM, and I found this formula efficiency=cpu_time / (run_time x number_of_cpus), so I am wondering if we should use this instead?

@skrakau
Copy link
Collaborator Author

skrakau commented Jan 17, 2024

Thanks!

But now I checked how to find cpu usage, according to the Green algorithms FAQ (How do I find the usage factor of my processors?) there are some commands such as seff for SLURM, and I found this formula efficiency=cpu_time / (run_time x number_of_cpus), so I am wondering if we should use this instead?

The Green algorithms docs are meant for usurs who manually have to provide and figure out those values. In our case we luckily have those metrics from Nextflow. And if I see it correctly, the seff CPU efficiency should be the same as what we compute based on the Nextflow cpu_usage.

@skrakau
Copy link
Collaborator Author

skrakau commented Jan 17, 2024

Where did you actually find this formula? Because I could find proper documentation

@mirpedrol
Copy link
Collaborator

Here is how the seff command calculates it.

@skrakau
Copy link
Collaborator Author

skrakau commented Jan 17, 2024

Here is how the seff command calculates it.

Thanks! I think it's in the end the same as using the cpu_usage from Nextflow, which also include the walltime and the CPU time for the calculation

@skrakau skrakau merged commit c213bff into nextflow-io:dev Jan 17, 2024
2 checks passed
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.

2 participants