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.

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

Fix gasnet-related test failures #6377

Merged
merged 5 commits into from
Nov 10, 2023
Merged

Fix gasnet-related test failures #6377

merged 5 commits into from
Nov 10, 2023

Conversation

Pansysk75
Copy link
Member

@Pansysk75 Pansysk75 commented Oct 30, 2023

Tests were broken (not running) due to an environment variable being set incorrectly.
@ct-clmsn Would this be ok?

@Pansysk75
Copy link
Member Author

retest lsu

@ct-clmsn
Copy link
Contributor

@Pansysk75 LGTM

@hkaiser
Copy link
Member

hkaiser commented Oct 30, 2023

@Pansysk75 Something is wrong here, see the failing tests, please.

set_tests_properties(
"${_full_name}" PROPERTIES RUN_SERIAL TRUE ENVIRONMENT
"GASNET_PSHM_NODES=2"
)
Copy link
Member

Choose a reason for hiding this comment

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

Should this environment variable added only if running using the gasnet parcelport?

@Pansysk75 Pansysk75 changed the title Fix test gasnet env variable Fix gasnet-related test failures Oct 31, 2023
@Pansysk75
Copy link
Member Author

@ct-clmsn
I get a GASNET_PSHM_NODES not specified: running with 1 process even on non-gasnet distributed tests. Should this env-variable be set for all distributed tests?
Also, on the gasnet tests, we get the following error:
FileNotFoundError: [Errno 2] No such file or directory: 'amudprun'
Is this even available on Rostam?
Thanks!

@ct-clmsn
Copy link
Contributor

ct-clmsn commented Oct 31, 2023

@Pansysk75 amudprun is a launcher provided by gasnet. It's compiled and installed with gasnet. I'd suggest checking to see if it's getting installed into ${CMAKE_INSTALL_PREFIX} and then checking if $PATH is getting updated to point to ${CMAKE_INSTALL_PREFIX}/bin. Below is amudprun --help to explain the arguments.

amudprun --help
<install_path>/bin/amudprun, version 3.19
Usage: <install_path>/bin/amudprun [<options>] <program_name> [<program arguments>...]

  -np N     Spawn N processes (may also be spelled -N or -n)
  -spawn F  Use spawning function F
  -depth D  Use network depth D
  -v        Enable verbose output, repeated use increases verbosity
  -h        Show this help

Available spawn functions:
    'S'  Spawn jobs using ssh remote shells
    'L'  Spawn jobs using fork()/exec() on the local machine (good for SMP's)
    'C'  Spawn jobs using custom job spawner (GASNET_CSPAWN_CMD)

HPX is throwing GASNET_PSHM_NODES errors b/c the gasnet parcelport is built as a shared library (just like the other parcelports - MPI, libfabric, udp, etc). When the parcelport shared library is loaded by HPX, the gasnet parcelport library is checking for this environment variable. That seems wrong since gasnet isn't getting initialized, I'll have to dig through gasnet to verify the behavior.

Copy link

codacy-production bot commented Oct 31, 2023

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.02%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (89f342b) 191587 163118 85.14%
Head commit (e098bb0) 188868 (-2719) 160842 (-2276) 85.16% (+0.02%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#6377) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@Pansysk75 Pansysk75 force-pushed the fix_tests_cmake branch 2 times, most recently from bb129d1 to a1edead Compare November 3, 2023 21:54
@hkaiser
Copy link
Member

hkaiser commented Nov 7, 2023

@Pansysk75 what's the status with this PR?

@Pansysk75
Copy link
Member Author

Pansysk75 commented Nov 8, 2023

@Pansysk75 what's the status with this PR?

Still in progress (no particular blockers), I'll have a look as soon as I get the opportunity.
IMPORTANT @hkaiser I must remind you that no tests are actually running until this is completed and merged!

@Pansysk75 Pansysk75 marked this pull request as ready for review November 9, 2023 23:40
@Pansysk75
Copy link
Member Author

@hkaiser I think we should merge this so that other PRs get tested properly (Assuming I didn't mess up anything else). Since gasnet might need some more work, I removed it from the testing for now, so that the tests give somewhat meaningful results.
We can tackle the remaining gasnet issues in a new PR.
Thanks a lot to @ct-clmsn for the help with this one

@hkaiser hkaiser merged commit 48d9f0e into master Nov 10, 2023
41 of 52 checks passed
@hkaiser hkaiser deleted the fix_tests_cmake branch November 10, 2023 00:43
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.

3 participants