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

Add CPU model name to trace files and traceRecord #3946

Merged
merged 8 commits into from
Jul 5, 2023

Conversation

skrakau
Copy link
Contributor

@skrakau skrakau commented May 15, 2023

This adds the CPU model name to the trace files (to allow using this information for a CO2 footprint plugin) and accordingly to the traceRecord.

It is not added to any tests yet.

@bentsherman
Copy link
Member

Wondering if we should integrate this with the arch directive somehow. See #3786

@pditommaso
Copy link
Member

I would expect the arch to be included in the CPU model

Copy link
Member

@pditommaso pditommaso left a comment

Choose a reason for hiding this comment

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

This looks ok, but there's a failing test. Likey due to new field

skrakau and others added 5 commits June 19, 2023 13:56
Signed-off-by: Sabrina Krakau <sabrinakrakau@gmail.com>
Signed-off-by: mirpedrol <mirp.julia@gmail.com>
Signed-off-by: Sabrina Krakau <sabrinakrakau@gmail.com>
Signed-off-by: Sabrina Krakau <sabrinakrakau@gmail.com>
Signed-off-by: Sabrina Krakau <sabrinakrakau@gmail.com>
@skrakau
Copy link
Contributor Author

skrakau commented Jun 20, 2023

Hi,
I moved the new cpu_model field to the end of the list, adjusted the test-bash-wrapper-with-trace.txt file to the new command-trace.txt since this caused an error in the BashWrapperBuilderTest test, and added an entry to the docs. I hope I didn't forget anything.

@skrakau skrakau marked this pull request as ready for review June 20, 2023 13:29
@skrakau skrakau requested a review from pditommaso June 21, 2023 13:13
@skrakau
Copy link
Contributor Author

skrakau commented Jun 27, 2023

This looks ok, but there's a failing test. Likey due to new field

Could you have another look at the recent changes @pditommaso ?

docs/tracing.md Outdated Show resolved Hide resolved
…oovy [ci fast]

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@netlify
Copy link

netlify bot commented Jul 5, 2023

Deploy Preview for nextflow-docs-staging ready!

Name Link
🔨 Latest commit fd6ca60
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/64a52f0f023e4f0008999ccc
😎 Deploy Preview https://deploy-preview-3946--nextflow-docs-staging.netlify.app/tracing
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Copy link
Member

We are ready to go! Thanks for this useful contribution!

@pditommaso pditommaso merged commit e0d91bf into nextflow-io:master Jul 5, 2023
5 checks passed
@skrakau
Copy link
Contributor Author

skrakau commented Jul 5, 2023

Thanks for adding it!

abhi18av pushed a commit to abhi18av/nextflow that referenced this pull request Oct 28, 2023
This commit add the `cpu_model` in the Nextflow trace record. 

Note: the field is not included in the default trace file, but it can be added by adding 
in the nextflow config the setting 

```
trace.fields = '..,cpu_model'
```


Signed-off-by: Sabrina Krakau <sabrinakrakau@gmail.com>
Co-authored-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
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.

4 participants