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.

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

test: detect netmod and enable netmod-specific tests #5981

Closed
wants to merge 3 commits into from
Closed

test: detect netmod and enable netmod-specific tests #5981

wants to merge 3 commits into from

Conversation

hzhou
Copy link
Contributor

@hzhou hzhou commented Apr 29, 2022 •
edited
Loading

Pull Request Description

Some tests, currently the test impls/mpich/threads/pt2pt/multinic_infohints will only work with ch4:ofi. Detect the netmod in configure and use macro to guard the test against other device or netmod.

Future TODO:

  • let #ifdefed out test to print Test Skipped and let runtest to recognize it.
    [skip warnings]

Author Checklist

  • Provide Description
    Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
  • Commits Follow Good Practice
    Commits are self-contained and do not do two things at once.
    Commit message is of the form: module: short description
    Commit message explains what's in the commit.
  • Passes All Tests
    Whitespace checker. Warnings test. Additional tests via comments.
  • Contribution Agreement
    For non-Argonne authors, check contribution agreement.
    If necessary, request an explicit comment from your companies PR approval manager.

@hzhou
Copy link
Contributor Author

hzhou commented Apr 29, 2022 •
edited
Loading

test:mpich/ch4/most
test:mpich/ch3/most
✔️

@hzhou
Copy link
Contributor Author

hzhou commented Apr 29, 2022

Added AC_MSG_NOTICE to log whether checking of netmod is successful

test:mpich/ch4/ofi

hzhou added 2 commits April 29, 2022 09:14
@hzhou
test/configure: probe mpich netmod and define macros
bc75941 
Define specific macros so we can have tests specific to certain netmod.
@hzhou
test: use macro to protect ch4:ofi only tests
cc116e3 
When MPICH_CH4_OFI is not define, the test reduce to a dummy test.
@hzhou
Copy link
Contributor Author

hzhou commented Apr 29, 2022 •
edited
Loading

test:mpich/ch4/ofi

EDIT: looks good

checking Is the MPI derived from MPICH... yes
configure: Detected MPICH with ch4:ofi netmod

@hzhou hzhou requested review from sagarth and yfguo April 29, 2022 14:31
@hzhou
Copy link
Contributor Author

hzhou commented Apr 29, 2022

test:mpich/ch4/most
test:mpich/ch3/most

@hzhou
test: enhance runtests to allow macro-skipped tests
651df29 
Some tests are skipped by checking configure macros. Let runtests to
recognize special output "Test Skipped" so we can tell from test report
whether a test is skipped or actually ran.
@hzhou
Copy link
Contributor Author

hzhou commented Apr 29, 2022 •
edited
Loading

test:mpich/ch4/most
test:mpich/ch3/most

Showing the test as skipped in e.g. ch4:ucx

image

And on ch4:ofi -

image

@hzhou
Copy link
Contributor Author

hzhou commented Apr 29, 2022

1 failure ./threads/pt2pt/mt_improbe_sendrecv_huge 2 -iter=64 -count=4194304 MPIR_CVAR_CH4_OFI_EAGER_MAX_MSG_SIZE=16384

Retest --
test:mpich/ch4/ofi

@hzhou
Copy link
Contributor Author

hzhou commented May 2, 2022

@raffenet Oops I accidentally merged #5946 which includes this one. Did you review this one? If changes are required, I'll make a new PR to fix it.

@raffenet
Copy link
Contributor

raffenet commented May 2, 2022

@raffenet Oops I accidentally merged #5946 which includes this one. Did you review this one? If changes are required, I'll make a new PR to fix it.

I think its probably fine. I don't know if using mpichversion would more reliable results, especially for older releases, since they might not support the new features anyway. Also, how would a multi-netmod build be treated?

@hzhou
Copy link
Contributor Author

hzhou commented May 2, 2022

@raffenet Oops I accidentally merged #5946 which includes this one. Did you review this one? If changes are required, I'll make a new PR to fix it.

I think its probably fine. I don't know if using mpichversion would more reliable results, especially for older releases, since they might not support the new features anyway. Also, how would a multi-netmod build be treated?

Hmm, I guess mpichversion will work. In my test build, I never build mpichversion thus it didn't occur to me. The check in this PR checks for runtime, so it should work with multi-netmod build. In addition, potentially it can check netmod features and selectively run specific tests.

If MPIR_CVAR_DEBUG_SUMMARY is not supported or it is missing the keywords the config is looking for, it will simply result in the tests not run. The tests probably won't run for older versions anyway.

@raffenet
Copy link
Contributor

raffenet commented May 3, 2022

If MPIR_CVAR_DEBUG_SUMMARY is not supported or it is missing the keywords the config is looking for, it will simply result in the tests not run. The tests probably won't run for older versions anyway.

I think the current method is fine, then. No need to change anything.

@hzhou
Copy link
Contributor Author

hzhou commented May 3, 2022

The commits in this PR has already been merged as part of #5946

@hzhou hzhou closed this May 3, 2022
@hzhou hzhou deleted the 2204_ofi_hint branch May 3, 2022 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sagarth sagarth Awaiting requested review from sagarth

@yfguo yfguo Awaiting requested review from yfguo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hzhou @raffenet