[feat] Improve framework messages and the performance report #2618

vkarak · 2022-10-01T22:10:31Z

This PR introduces several small improvements that imo enhance the daily experience.

The log files are printed before the run start. Therefore it is easy to inspect the logs live for long runs, especially with the default log configuration, which creates a temporary log file.
The performance data of a performance test is printed immediately after the test finishes (either successfully or not). This is especially useful for long performance exploration experiments, where you don't want to wait until the end of the run to see the performance in the performance report.
The performance report is modernised and improved. More information is listed but in a more compact and concise way.
All error and warning messages are prefixed with ERROR and WARNING always. This makes it easier spot errors and warnings, even when no colors are used.
Some debug messages where moved to debug2.
A warning when building the docs was fixed.
The docs and local run listings are updated.

TODOS

Update the remote run listings (@ekouts Could you do that on Daint?)

- Print `ERROR` and `WARNING` in errors and warnings - Move a selecting subconfig message to `debug2` - Print the log file info always at INFO level

- Text + listings

codecov-commenter · 2022-10-01T22:13:16Z

Codecov Report

Base: 86.69% // Head: 86.27% // Decreases project coverage by -0.42% ⚠️

Coverage data is based on head (6c51b54) compared to base (9f0d167).
Patch coverage: 93.22% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2618      +/-   ##
==========================================
- Coverage   86.69%   86.27%   -0.43%     
==========================================
  Files          59       60       +1     
  Lines       10868    10977     +109     
==========================================
+ Hits         9422     9470      +48     
- Misses       1446     1507      +61

Impacted Files	Coverage Δ
reframe/frontend/cli.py	`72.23% <75.00%> (-0.07%)`	⬇️
reframe/frontend/statistics.py	`95.94% <92.85%> (+0.09%)`	⬆️
reframe/core/config.py	`91.53% <100.00%> (ø)`
reframe/core/logging.py	`77.77% <100.00%> (ø)`
reframe/frontend/executors/policies.py	`92.50% <100.00%> (+0.19%)`	⬆️
reframe/core/systems.py	`89.95% <0.00%> (-1.40%)`	⬇️
reframe/core/environments.py	`96.39% <0.00%> (-0.91%)`	⬇️
reframe/core/schedulers/flux.py	`33.33% <0.00%> (ø)`
reframe/core/backends.py	`93.93% <0.00%> (+3.93%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

reframe/frontend/statistics.py

docs/listings/hello2_list_verbose.txt

docs/listings/hellomp2.txt

reframe/frontend/cli.py

reframe/frontend/statistics.py

ekouts · 2022-10-05T12:36:40Z

I was trying to run the listings on daint and realized that the performance report is not reporting all the progenvs. This is the new performance report from alltests_daint:

[ReFrame Setup]
  version:           4.0.0-dev.1+6c51b548
...
[ RUN      ] HelloThreadedExtended2Test /57223829 @daint:mc+cray
[ RUN      ] StreamWithRefTest /f925207b @daint:login+gnu
[ RUN      ] StreamWithRefTest /f925207b @daint:gpu+gnu
[ RUN      ] StreamWithRefTest /f925207b @daint:mc+gnu
...
[       OK ] (16/42) StreamWithRefTest /f925207b @daint:login+gnu
P: Copy: 69777.8 MB/s (r:0, l:None, u:None)
P: Scale: 42953.6 MB/s (r:0, l:None, u:None)
P: Add: 46951.0 MB/s (r:0, l:None, u:None)
P: Triad: 47320.6 MB/s (r:0, l:None, u:None)
[       OK ] (17/42) HelloMultiLangTest %lang=cpp /71bf65a3 @daint:mc+gnu
...
[       OK ] (25/42) StreamWithRefTest /f925207b @daint:mc+gnu
P: Copy: 49051.3 MB/s (r:0, l:None, u:None)
P: Scale: 31809.1 MB/s (r:0, l:None, u:None)
P: Add: 33709.2 MB/s (r:0, l:None, u:None)
P: Triad: 33932.1 MB/s (r:0, l:None, u:None)
[       OK ] (26/42) HelloMultiLangTest %lang=c /7cfa870e @daint:gpu+cray
...
[       OK ] (41/42) StreamWithRefTest /f925207b @daint:gpu+gnu
P: Copy: 50739.1 MB/s (r:0, l:None, u:None)
P: Scale: 34674.5 MB/s (r:0, l:None, u:None)
P: Add: 38353.3 MB/s (r:0, l:None, u:None)
P: Triad: 38683.2 MB/s (r:0, l:None, u:None)
[       OK ] (42/42) HelloThreadedExtended2Test /57223829 @daint:gpu+nvidia
[----------] all spawned checks have finished

[  PASSED  ] Ran 42/42 test case(s) from 4 check(s) (0 failure(s), 0 skipped)
[==========] Finished on Wed Oct  5 14:22:30 2022 

================================================================================
PERFORMANCE REPORT
--------------------------------------------------------------------------------
[StreamWithRefTest /f925207b @daint:mc:gnu]
  num_gpus_per_node: 0
  num_tasks: 1
  performance:
    - Copy: 49051.3 MB/s (r: 0 MB/s l: -inf% u: +inf%)
    - Scale: 31809.1 MB/s (r: 0 MB/s l: -inf% u: +inf%)
    - Add: 33709.2 MB/s (r: 0 MB/s l: -inf% u: +inf%)
    - Triad: 33932.1 MB/s (r: 0 MB/s l: -inf% u: +inf%)
--------------------------------------------------------------------------------
Run report saved in '/home/user/.reframe/reports/run-report-59.json'
Log file(s) saved in '/tmp/rfm-vju06n8f.log'

ekouts · 2022-10-05T14:03:51Z

I have updated the remote listings, but we need to re-run alltests_daint.txt and stream4_daint.txt after the problem with the performance report is fixed. Please let me know if I missing any other listing.

I also needed to change some tests because the nvidia environment of daint needed a fix. I didn't push the changes because I think they may complicate the tutorial for no reason. What do you think? I needed to add this in a couple of tests.

    @run_before('compile')
    def prgenv_nvidia_workaround(self):
        ce = self.current_environ.name
        if ce == 'nvidia':
            self.build_system.cppflags += [
                '-D__GCC_ATOMIC_TEST_AND_SET_TRUEVAL'
            ]

vkarak · 2022-10-05T20:35:36Z

I also needed to change some tests because the nvidia environment of daint needed a fix. I didn't push the changes because I think they may complicate the tutorial for no reason. What do you think? I needed to add this in a couple of tests.

Good point, it will complicate it without a reason, but perhaps this deserves a nice small tip-n-trick section, where we can show the functionality of disabling the workaround hook. ;-)

vkarak · 2022-10-05T20:36:05Z

I was trying to run the listings on daint and realized that the performance report is not reporting all the progenvs. This is the new performance report from alltests_daint:

Hmm, that smells like a bug. I will have a look at it.

vkarak · 2022-10-05T20:37:11Z

Btw, what do you think about the P: lines print outs? Should they be in the verbose mode?

ekouts · 2022-10-06T07:49:24Z

Btw, what do you think about the P: lines print outs? Should they be in the verbose mode?

I honestly don't know what I prefer. Initially I thought about asking to put it in verbose, but I kinda like it and find it useful. I think most people don't run in verbose mode and wouldn't see it probably. But again, I don't have a strong opinion on this.

Good point, it will complicate it without a reason, but perhaps this deserves a nice small tip-n-trick section, where we can show the functionality of disabling the workaround hook. ;-)

Hm how about I put it under # rfmdocend: and then we show it only in the tips-and-tricks section?

vkarak · 2022-10-06T14:18:15Z

I honestly don't know what I prefer. Initially I thought about asking to put it in verbose, but I kinda like it and find it useful. I think most people don't run in verbose mode and wouldn't see it probably. But again, I don't have a strong opinion on this.

I had the same feelings as well, that's why I asked. I'm glad that we agree! I kinda prefer it in the non-verbose mode as well.

Hm how about I put it under # rfmdocend: and then we show it only in the tips-and-tricks section?

Yes, if that works, that'd be good!

victorusu

lgtm

reframe/frontend/statistics.py

ekouts

Some very minor comments. deps_rerun_t6 and deps_rerun_t6 should be run without config file according to gendoclistings. We should update the listingsa or the gendoclistings.py

docs/listings/deps_rerun_t6.txt

docs/listings/deps_run_t6.txt

vkarak · 2022-10-07T22:02:24Z

@ekouts The tutorial looks good to me; I just did a little bit of refinement.

vkarak added 9 commits September 29, 2022 23:18

Modernize performance report

2e43d4f

Update rerun message in failure report

c815d06

Improve log messages

621c481

- Print `ERROR` and `WARNING` in errors and warnings - Move a selecting subconfig message to `debug2` - Print the log file info always at INFO level

Fix documentation warning

3f97073

Print performance numbers immediately

251ad1c

Print the log file names before starting the run

4a7027f

Fix PEP8 issues

4a46371

Fix unit tests

36ffa08

Update docs

6c51b54

- Text + listings

vkarak added prio: normal enhancement labels Oct 1, 2022

vkarak requested review from ekouts and victorusu October 1, 2022 22:10

vkarak self-assigned this Oct 1, 2022

vkarak changed the title ~~[feat] Improve framework message and the performance report~~ [feat] Improve framework messages and the performance report Oct 3, 2022

vkarak requested a review from teojgo October 4, 2022 17:16

ekouts requested changes Oct 5, 2022

View reviewed changes

Upload remote listings

ccca5d8

Address PR comments

6018118

victorusu approved these changes Oct 6, 2022

View reviewed changes

Add tutorial for --disable-hook

a4a2a15

teojgo approved these changes Oct 7, 2022

View reviewed changes

ekouts requested changes Oct 7, 2022

View reviewed changes

reframe/frontend/statistics.py Show resolved Hide resolved

ekouts approved these changes Oct 7, 2022

View reviewed changes

Add alltests_daint.txt and stream4_daint.txt

3b06b11

ekouts requested changes Oct 7, 2022

View reviewed changes

docs/listings/deps_rerun_t6.txt Outdated Show resolved Hide resolved

docs/listings/deps_run_t6.txt Outdated Show resolved Hide resolved

vkarak added 2 commits October 7, 2022 23:07

Fix deps example listing

9d506e1

Update tutorial about disabling hooks

e562931

ekouts approved these changes Oct 8, 2022

View reviewed changes

Merge branch 'master' into feat/improve-messages

224cbf0

ekouts merged commit c15a073 into reframe-hpc:master Oct 8, 2022

vkarak mentioned this pull request Oct 13, 2022

[bugfix] Get performance info from within the changedir #2625

Closed

vkarak deleted the feat/improve-messages branch February 3, 2023 20:25

[feat] Improve framework messages and the performance report #2618

[feat] Improve framework messages and the performance report #2618

Uh oh!

Conversation

vkarak commented Oct 1, 2022 • edited by ekouts Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Oct 1, 2022

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ekouts commented Oct 5, 2022

Uh oh!

ekouts commented Oct 5, 2022

Uh oh!

vkarak commented Oct 5, 2022

Uh oh!

vkarak commented Oct 5, 2022

Uh oh!

vkarak commented Oct 5, 2022

Uh oh!

ekouts commented Oct 6, 2022

Uh oh!

vkarak commented Oct 6, 2022

Uh oh!

victorusu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ekouts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vkarak commented Oct 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vkarak commented Oct 1, 2022 •

edited by ekouts

Loading