Skip to content

[test] Convert GpuBandwidthCheck to a flexible diagnostic check #586

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Dec 10, 2018
Merged

[test] Convert GpuBandwidthCheck to a flexible diagnostic check #586

merged 9 commits into from
Dec 10, 2018

Conversation

@ajocksch
Copy link
Contributor

@ajocksch ajocksch commented Nov 16, 2018 •
edited by vkarak
Loading

Closes #578.

@ajocksch ajocksch self-assigned this Nov 16, 2018
@ajocksch ajocksch requested review from teojgo and vkarak November 16, 2018 17:03
@vkarak vkarak changed the title WIP: green on dom; still needs debugging [WIP] green on dom; still needs debugging Nov 16, 2018
@vkarak
Copy link
Contributor

vkarak commented Nov 16, 2018

@ajocksch Can you please put more precise names in your PRs? And optionally a description of what they are addressing? The titles of PRs are important, cos they are used for generating the release notes.

@vkarak vkarak added this to the ReFrame sprint 2018w44 milestone Nov 16, 2018
@vkarak
Copy link
Contributor

vkarak commented Nov 18, 2018

@ajocksch Flexible tests must not be production.

@ajocksch ajocksch changed the title [WIP] green on dom; still needs debugging [WIP] extension of GpuBandwidthCheck to flexible nodecount Nov 21, 2018
@vkarak vkarak changed the title [WIP] extension of GpuBandwidthCheck to flexible nodecount [WIP] Extend GpuBandwidthCheck to flexible nodecount Nov 21, 2018
@vkarak vkarak changed the title [WIP] Extend GpuBandwidthCheck to flexible nodecount [WIP] Extend GpuBandwidthCheck to use flexible node allocation Nov 21, 2018
@ajocksch ajocksch changed the title [WIP] Extend GpuBandwidthCheck to use flexible node allocation [WIP] extend GpuBandwidthCheck to flexible nodecount Nov 21, 2018
@vkarak vkarak changed the title [WIP] extend GpuBandwidthCheck to flexible nodecount [WIP] Extend GpuBandwidthCheck to use flexible node allocation Nov 21, 2018
@codecov-io
Copy link

codecov-io commented Nov 21, 2018 •
edited
Loading

Codecov Report

Merging #586 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #586      +/-   ##
==========================================
+ Coverage   91.57%   91.59%   +0.02%     
==========================================
  Files          72       72              
  Lines        8980     8980              
==========================================
+ Hits         8223     8225       +2     
+ Misses        757      755       -2
Impacted Files Coverage Δ
reframe/core/config.py 84.34% <0%> (+1.73%) ⬆️

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 b449c24...0146139. Read the comment docs.

@ajocksch
Copy link
Contributor Author

ajocksch commented Nov 21, 2018

the production label still needs to be removed; debugging in the sense that the multi-line regexpr should cover also lines mixed between different nodes -- the current tests did not produce this

vkarak
vkarak requested changes Nov 24, 2018
View reviewed changes
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.

Apart from my style comments and suggestions, the sanity checking implementation seems unnecessarily complicated. What do you want to achieve exactly?

@vkarak vkarak self-assigned this Nov 26, 2018
@vkarak
Copy link
Contributor

vkarak commented Nov 26, 2018

Also I think the flexible test can be combine with the non-flexible one. In my opinion it should replace it and removed from production.

@vkarak vkarak changed the title [WIP] Extend GpuBandwidthCheck to use flexible node allocation [WIP] [test] Extend GpuBandwidthCheck to use flexible node allocation Dec 1, 2018
@vkarak
Copy link
Contributor

vkarak commented Dec 1, 2018

@ajocksch This looks still as in work in progress. Let me know when I should review it again by removing the [WIP] tag.

@ajocksch ajocksch changed the title [WIP] [test] Extend GpuBandwidthCheck to use flexible node allocation [test] Extend GpuBandwidthCheck to use flexible node allocation Dec 1, 2018
Fix flexible GPU bandwidth test
94d3a48 
- Use a single programming environment
- Fix performance logging
- Log performance per node and GPU
- Implementation fine tuning
@pep8speaks
Copy link

pep8speaks commented Dec 9, 2018

Hello @ajocksch! Thanks for updating the PR.

@vkarak vkarak changed the title [test] Extend GpuBandwidthCheck to use flexible node allocation [test] Convert GpuBandwidthCheck to a flexible diagnostic check Dec 9, 2018
@vkarak vkarak merged commit 16563ff into reframe-hpc:master Dec 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vkarak vkarak vkarak approved these changes

@teojgo teojgo Awaiting requested review from teojgo

Assignees

@vkarak vkarak

@ajocksch ajocksch

Labels

prio: important

Projects

None yet

Milestone

ReFrame sprint 2018w48

Development

Successfully merging this pull request may close these issues.

4 participants

@ajocksch @vkarak @codecov-io @pep8speaks