Skip to content

Conversation

@ekouts
Copy link
Contributor

@ekouts ekouts commented Nov 4, 2020

Fixes #1521.

Some output examples:

- OSUAllreduceTest_4:
    Description:
      OSU Allreduce test

    Environment modules:
      <none>

    Location:
      /users/karakasv/Devel/reframe/tutorials/deps/osu_benchmarks.py

    Maintainers:
      <none>

    Node allocation:
      standard (4 task(s))

    Pipeline hooks:
      - post_setup: set_executable

    Tags:
      <none>

    Valid environments:
      gnu, pgi, intel

    Valid systems:
      daint:gpu

    Dependencies (conceptual):
      OSUBuildTest

    Dependencies (actual):
      - ('OSUAllreduceTest_4', 'daint:gpu', 'gnu') -> ('OSUBuildTest', 'daint:login', 'gnu')
      - ('OSUAllreduceTest_4', 'daint:gpu', 'intel') -> ('OSUBuildTest', 'daint:login', 'intel')
      - ('OSUAllreduceTest_4', 'daint:gpu', 'pgi') -> ('OSUBuildTest', 'daint:login', 'pgi')
- OSUDownloadTest:
    Description:
      OSU benchmarks download sources

    Environment modules:
      <none>

    Location:
      /Users/karakasv/Repositories/reframe/tutorials/deps/osu_benchmarks.py

    Maintainers:
      <none>

    Node allocation:
      standard (1 task(s))

    Pipeline hooks:

    Tags:
      <none>

    Valid environments:
      gnu

    Valid systems:
      daint:login

    Dependencies (conceptual):
      <none>

    Dependencies (actual):
      <none>

I kept the old Dependencies because the check itself might have dependencies to more checks than will appear on this specific system.

@ekouts ekouts requested review from teojgo and vkarak November 4, 2020 13:49
@ekouts ekouts self-assigned this Nov 4, 2020
@pep8speaks
Copy link

pep8speaks commented Nov 4, 2020

Hello @ekouts, Thank you for updating!

Cheers! There are no PEP8 issues in this Pull Request!Do see the ReFrame Coding Style Guide

Comment last updated at 2020-11-07 15:01:35 UTC

@ekouts
Copy link
Contributor Author

ekouts commented Nov 4, 2020

@jenkins-cscs retry all

@codecov-io
Copy link

codecov-io commented Nov 4, 2020

Codecov Report

Merging #1576 (4c7b707) into master (8982bdb) will decrease coverage by 0.00%.
The diff coverage is 89.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1576      +/-   ##
==========================================
- Coverage   88.02%   88.02%   -0.01%     
==========================================
  Files          45       45              
  Lines        7016     7030      +14     
==========================================
+ Hits         6176     6188      +12     
- Misses        840      842       +2     
Impacted Files Coverage Δ
reframe/frontend/cli.py 79.75% <89.47%> (+0.21%) ⬆️

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 8982bdb...4c7b707. Read the comment docs.

@vkarak
Copy link
Contributor

vkarak commented Nov 4, 2020

What I can't understand from the output samples you posted is why the "Dependencies for current system" are empty for the second set of examples, when you have for example test dependencies.

@teojgo
Copy link
Contributor

teojgo commented Nov 4, 2020

What I can't understand from the output samples you posted is why the "Dependencies for current system" are empty for the second set of examples, when you have for example test dependencies.

Executing this:
./bin/reframe -C unittests/resources/settings.py --system=sys0 -c unittests/resources/checks_unlisted/deps_simple.py -L

looks good for me.

@ekouts
Copy link
Contributor Author

ekouts commented Nov 4, 2020

What I can't understand from the output samples you posted is why the "Dependencies for current system" are empty for the second set of examples, when you have for example test dependencies.

@vkarak The environments e0 and e1 are not in the configuration, so no test is going to run in the end

@vkarak
Copy link
Contributor

vkarak commented Nov 4, 2020

@vkarak The environments e0 and e1 are not in the configuration, so no test is going to run in the end

This is a bit problematic and comes from the fact that the printing is based on tests not on the test cases. I was about to comment something related to that for your PR. Perhaps we can address also this problem. If the tests are not going to run, they should not be listed.

@ekouts
Copy link
Contributor Author

ekouts commented Nov 4, 2020

This is a bit problematic and comes from the fact that the printing is based on tests not on the test cases. I was about to comment something related to that for your PR. Perhaps we can address also this problem. If the tests are not going to run, they should not be listed.

Okay, I can easily print only the checks that are included in the index_of_deps. Or do you prefer a different way to solve this issue?

@vkarak
Copy link
Contributor

vkarak commented Nov 4, 2020

I don't like the index_of_deps concept being computed outside and then just being passed along all the way down to format_checks(). The interface doesn't make sense. The list_checks() should take the test cases instead and derive the actual tests and dependencies from there. You build the dependency index privately inside that function or wherever it is actually needed.

@ekouts
Copy link
Contributor Author

ekouts commented Nov 4, 2020

Okay, I think I can pass only the testcases and infer the checks from there. This way we will only have the checks that will run

Vasileios Karakasis added 3 commits November 7, 2020 15:46
- Do not pass `index_of_deps` in `format_checks()` but rather just the list of
  dependencies for the actual test.
- Rename `index_of_deps` to simply `deps`.
- Slight changes in the output.
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.

I did some small changes. Lgtm now.

@vkarak vkarak changed the title [feat] Add a more detailed listing of dependencies [feat] Print more information about dependencies in detailed test listing Nov 7, 2020
@vkarak vkarak merged commit 58cfb68 into reframe-hpc:master Nov 7, 2020
@ekouts ekouts deleted the feat/detailed_deps branch November 16, 2020 13:16
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.

More detailed information about dependencies

5 participants