Fix profiler and TAU harness checks - #181
Conversation
Make profiler and TAU tests assert real artifacts instead of matching diagnostic text, fail TAU builds explicitly, detect MPI by querying the wrapper that will actually be used, and check the rocprof-compute dependency pins before a long profiling run rather than after it.
| endif | ||
|
|
||
| IS_OMPI=$(shell which ompi_info 2>/dev/null) | ||
| # Ask the wrapper we are about to invoke. `which ompi_info` reports that some |
There was a problem hiding this comment.
please revise these comments, if we remove the line that has ompi_info then it does not make sense to mention it in a comment. please revies all comments in general to make sure they add something useful, if not remove thanks
| source ${PROFILER_TOP_DIR}/setup.sh | ||
|
|
||
| # Dependency precondition. `analyze` enforces the exact pins in its own | ||
| # requirements.txt and this run ends in `analyze`, so verify them before |
There was a problem hiding this comment.
make sure to test that the changes you are proposing here do not break anything on aac6: run this branch there to make sure these python stuff works there too. rocprof-compute + python can be tricky. thanks
| set_property(TEST Rocprof-sys_ROCm_Run_Check PROPERTY SKIP_REGULAR_EXPRESSION "module spider") | ||
|
|
||
| add_test(NAME Rocprof-sys_ROCm_Stream_Overlap COMMAND ../rocprof-sys_stream_overlap.sh ) | ||
| set_property(TEST Rocprof-sys_ROCm_Stream_Overlap PROPERTY PASS_REGULAR_EXPRESSION "proto") |
There was a problem hiding this comment.
as you see in the other tests, we want the tests to have a twofold purpose: 1 be used to check that the installation is working, that is why they are part of the test suite. 2 make sure that when a user runs them standalone they can still get the information they need so we don't want the tests to just output a "PASSED" message, we want the test to also print out the configuration and parameters someone would be interested in seeing. please make sure your changes still allow this, thanks
| # unions. Fail here, naming the reason, rather than later with a SyntaxError | ||
| # from inside the tool that names no cause. | ||
| if ! python3 -c 'import sys; sys.exit(0 if sys.version_info >= (3,10) else 1)' 2>/dev/null; then | ||
| echo "ERROR: rocprof-compute needs Python >= 3.10, but python3 is $(python3 -V 2>&1)." |
There was a problem hiding this comment.
double check this claim please
| # directory: going to /tmp instead | ||
| # [proxy:1@node2] launch_procs: unable to change wdir to /tmp/.../build_ewLCPv | ||
| # | ||
| # The test wants two ranks, not two nodes -- Jacobi's "-g 2 1" topology on two |
There was a problem hiding this comment.
please rephrase these AI generated comments in human lingo and maybe trim a bit if it applies
gcapodagAMD
left a comment
gcapodagAMD
left a comment
There was a problem hiding this comment.
thanks @dannybaths please take a look at the comments I left in the files.
2 participants
Summary
This PR fixes several profiler and TAU test harness issues found while validating the AAC7 OpenMPI row.
corresponding CTest pass expressions are updated to match.
ompi_infoexists somewhere on
PATH.directory.
analyzeenforces before profiling,and the rocprof-compute tests fail immediately, with the reason, when Python is older than 3.10.
These are reported as failures rather than skips deliberately, so the failure count is not quietly
reduced.
Why
Several tests could pass or fail for the wrong reason:
profile.0is a substring ofCould not open profile.0.0.0, andprotoappears in output the tool prints when it producesnothing.
makefailed, so the final verdict depended only on regex output.Because
PASS_REGULAR_EXPRESSIONoverrides the exit code, the script and the CMake criteria had tobe fixed together.
ompi_infowas visible butmpic++hadbeen replaced by MPICH wrappers.
analyzestep thatcould not run, turning a dependency problem into a timeout that named no cause.
Validation
Validated on AAC7 with the OpenMPI PrgEnv row: MI300A, ROCm 7.14.0, OpenMPI 5.0.10 over OFI/CXI.
pass:
Rocprof-sys_ROCm_Stream_Overlap,Rocprof-compute_ROCm_Roofline_Check,TAU_Trace_Check,TAU_Profile_Check,TAU_Check_HIP_ProfileandTAU_Check_MPI_Profile. Each now matches on astring only the success path can produce.
test fails in about three seconds, reporting the version it found and the version it needs. Under
Python 3.12.12 it runs to completion and the roofline assertion finds a real
roofline.csv. Thefailing case is deliberately a failure rather than a skip, so an environment that cannot run
rocprof-compute is not quietly removed from the failure count.
tests build and produce real
profile.*/tautrace.*artifacts andpprofoutput containing MPIand HIP routines.
node-local
/tmp, but rank 1 was launched on the second node where that directory did not exist. ThisPR fixes that launch assumption too, by keeping the ranks on the node that owns the build directory.
Verified on two nodes with the checkout in node-local
/tmp— the failure reproduces without the fixand both invocations pass with it — and then in the full two-node suite, where all four Jacobi TAU
tests pass on real artifacts (319 pass / 27 fail / 15 skip / 1 timeout of 362, suspicious-pass 0).
the OpenMPI-built
rccl-testsbenchmark. This PR does not attempt to solve that packaging mismatch.harness bug.