Skip to content

Conversation

@ekouts
Copy link
Contributor

@ekouts ekouts commented Feb 14, 2022

It might be better to remove the runtime timings from the default output, after the conversation in #2415 .
The documentation mentions that these timings correspond only to the ReFrame stages, but it can be misleading for new users that want to use them for performance monitoring.
Normal output:

[----------] start processing checks
[ RUN      ] T0 @generic:default+builtin
[       OK ] ( 1/10) T0 @generic:default+builtin
[ RUN      ] T4 @generic:default+builtin
[       OK ] ( 2/10) T4 @generic:default+builtin

Verbose output:

[----------] start processing checks
[ RUN      ] T0 @generic:default+builtin
[       OK ] ( 1/10) T0 @generic:default+builtin
==> setup: 0.019s compile: 0.035s run: 0.476s sanity: 0.000s performance: 0.000s total: 0.568s
[ RUN      ] T4 @generic:default+builtin
[       OK ] ( 2/10) T4 @generic:default+builtin
==> setup: 0.009s compile: 0.036s run: 0.375s sanity: 0.000s performance: 0.000s total: 0.459s

I still leave them in verbose mode, what do you think @vkarak ? If you agree I will change also all the reframe outputs in the docs.

Closes #2415.

@ekouts ekouts added this to the ReFrame Sprint 22.02.1 milestone Feb 14, 2022
@ekouts ekouts requested a review from vkarak February 14, 2022 10:53
@ekouts ekouts self-assigned this Feb 14, 2022
@vkarak vkarak changed the title Remove runtime timings from the default output [feat] Remove runtime timings from the default output Feb 14, 2022
@vkarak
Copy link
Contributor

vkarak commented Feb 14, 2022

Should we also close #2415 with this?

@ekouts
Copy link
Contributor Author

ekouts commented Feb 15, 2022

I think so, yes.

@victorusu victorusu self-requested a review February 15, 2022 13:08
Copy link
Contributor

@victorusu victorusu left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

Lgtm, I have a minor comment for improvement.

@codecov-commenter
Copy link

Codecov Report

Merging #2425 (b72b7eb) into master (e2843b0) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2425      +/-   ##
==========================================
- Coverage   85.67%   85.66%   -0.01%     
==========================================
  Files          56       56              
  Lines       10466    10460       -6     
==========================================
- Hits         8967     8961       -6     
  Misses       1499     1499              
Impacted Files Coverage Δ
reframe/frontend/executors/policies.py 92.41% <100.00%> (-0.13%) ⬇️

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 e2843b0...b72b7eb. Read the comment docs.

@vkarak vkarak merged commit 0240024 into reframe-hpc:master Feb 15, 2022
@ekouts ekouts deleted the remove_runtime_stats branch February 16, 2022 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test runtime statistics with incorrect run and total values

4 participants