Add compute to compute event handoff test#27
Add compute to compute event handoff test#27psychocoderHPC wants to merge 1 commit intopr-add-compute-to-transfer-event-handoff-testfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/integ/computeToComputeEventHandoff/computeToComputeEventHandoff.cpp`:
- Around line 1-3: Update the file header in computeToComputeEventHandoff.cpp to
match the repository's standard provenance format used across tests (replace the
current "Copyright 2026 OpenAI" lines with the project-standard header used in
sibling files like computeToTransferEventHandoff.cpp and
queueBufferRoundTrip.cpp), ensuring the SPDX license token remains MPL-2.0 and
the year/contributor attribution follows the repository template.
- Around line 50-53: The code currently silently returns when no device is
available in the block that checks devSelector.isAvailable(), which makes the
test appear passed; replace the early return with a call to SKIP(...) so the
test is marked skipped: call SKIP(...) with a message that includes
deviceSpec.getName() (e.g., SKIP("No device available for " <<
deviceSpec.getName())), leaving the devSelector.isAvailable() check and
surrounding logic intact.
- Around line 57-58: The test uses blocking host queues which causes
producerQueue.enqueue(WriteIntermediateKernel) to block the host until
completion, so the recorded event (intermediateReady) is already signaled before
consumerQueue.waitFor(intermediateReady) runs; update the test to use
non-blocking queues by changing device.makeQueue(queueKind::blocking) to
device.makeQueue(queueKind::nonBlocking) for the producer (or both queues) or
alternatively run producerQueue.enqueue(...) and
consumerQueue.waitFor(intermediateReady) from separate host threads so the event
handoff between onHost::Queue instances is actually exercised; ensure references
to onHost::Queue, device.makeQueue, queueKind::nonBlocking,
producerQueue.enqueue(WriteIntermediateKernel), intermediateReady, and
consumerQueue.waitFor are adjusted accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3a3ca86c-d13d-4d8a-82cf-dbfdcb5e3e48
📒 Files selected for processing (1)
test/integ/computeToComputeEventHandoff/computeToComputeEventHandoff.cpp
| /* Copyright 2026 OpenAI | ||
| * SPDX-License-Identifier: MPL-2.0 | ||
| */ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect nearby tests to use the repo's standard notice, or no notice at all.
echo "Nearby event-handoff tests:"
fd -t f -i 'eventhandoff' test | while read -r f; do
echo "==> $f"
sed -n '1,4p' "$f"
echo
done
echo "Representative copyright / SPDX headers in the repo:"
rg -n -m 20 'SPDX-License-Identifier|Copyright' || trueRepository: psychocoderHPC/alpaka3
Length of output: 50378
Align the file header provenance with the repository's standard.
Lines 1–2 assign copyright to "OpenAI" instead of individual contributors or the project's standard header format. This same pattern appears in related test files (computeToTransferEventHandoff.cpp, queueBufferRoundTrip.cpp), creating an inconsistency with the repository's normal attribution practice. Correct the header before merging to ensure proper provenance metadata.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/integ/computeToComputeEventHandoff/computeToComputeEventHandoff.cpp`
around lines 1 - 3, Update the file header in computeToComputeEventHandoff.cpp
to match the repository's standard provenance format used across tests (replace
the current "Copyright 2026 OpenAI" lines with the project-standard header used
in sibling files like computeToTransferEventHandoff.cpp and
queueBufferRoundTrip.cpp), ensuring the SPDX license token remains MPL-2.0 and
the year/contributor attribution follows the repository template.
| if(!devSelector.isAvailable()) | ||
| { | ||
| std::cout << "No device available for " << deviceSpec.getName() << std::endl; | ||
| return; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Catch2 references:"
rg -n -C2 'catch2/|Catch2\b' || true
echo
echo "Existing runtime-skip usage in tests:"
rg -n -C2 '\bSKIP\s*\(|\bCATCH_SKIP\s*\(' test || true
echo
echo "Other device-availability guards in tests:"
rg -n -C3 'isAvailable\(\)|No device available' test || trueRepository: psychocoderHPC/alpaka3
Length of output: 50378
🏁 Script executed:
# Check the exact file and lines referenced
head -60 test/integ/computeToComputeEventHandoff/computeToComputeEventHandoff.cpp | tail -20Repository: psychocoderHPC/alpaka3
Length of output: 738
🏁 Script executed:
# Search for Catch2 skip/skip-related functionality in includes
rg -n 'skip|SKIP' test/integ/computeToComputeEventHandoff/computeToComputeEventHandoff.cppRepository: psychocoderHPC/alpaka3
Length of output: 48
🏁 Script executed:
# Check Catch2 headers available in the codebase for skip functionality
find . -name "*.hpp" -path "*/catch2/*" | head -20Repository: psychocoderHPC/alpaka3
Length of output: 48
🏁 Script executed:
# Search for any skip/SKIP related content in Catch2 includes that might be present
rg -i 'skip' --include="*.hpp" | grep -i catch | head -20Repository: psychocoderHPC/alpaka3
Length of output: 507
🏁 Script executed:
# Check what Catch2 macros are actually used in similar test patterns
rg -n 'INFO|WARN|REQUIRE|CHECK' test/integ/computeToComputeEventHandoff/computeToComputeEventHandoff.cppRepository: psychocoderHPC/alpaka3
Length of output: 134
🏁 Script executed:
# Look for Catch2 session or skip functionality in the codebase
rg -n 'catch.*skip|skip.*catch' -i test/ --include="*.hpp" --include="*.cpp" | head -10Repository: psychocoderHPC/alpaka3
Length of output: 507
🌐 Web query:
Catch2 v3 skip test runtime SKIP macro
💡 Result:
In Catch2 v3, you can skip a test at runtime from inside the test body using the SKIP macro (added in Catch2 3.3.0) [1], [2].
`#include` <catch2/catch_test_macros.hpp>
TEST_CASE("needs special hardware") {
if (!has_required_feature()) {
SKIP("required feature not available on this machine");
}
// rest of test...
}Key points:
- Signature:
SKIP([streamable expression])(message optional) [1]. - Stops execution immediately (like an explicit
FAIL)—code after it won’t run [1]. - If you already had a failing assertion earlier in the test (e.g.
CHECKfailed), the test case is still reported as failed, not skipped [1]. - If all test cases end up skipped, Catch2 returns a non-zero exit code unless you pass
--allow-running-no-tests[1], [3].
Sources: [1] [2] [3]
🏁 Script executed:
# Look for Catch2 macros in the includes being used
rg -n 'catch2/catch' test/integ/computeToComputeEventHandoff/computeToComputeEventHandoff.cppRepository: psychocoderHPC/alpaka3
Length of output: 158
🏁 Script executed:
# Search for any test macros defined or used in test files
rg 'TEMPLATE_LIST_TEST_CASE|TEST_CASE' test/integ/computeToComputeEventHandoff/computeToComputeEventHandoff.cpp -A 15 | head -30Repository: psychocoderHPC/alpaka3
Length of output: 675
🏁 Script executed:
# Check if there are other device unavailability checks in the same file that need fixing
rg -n 'isAvailable()' test/integ/computeToComputeEventHandoff/computeToComputeEventHandoff.cpp -B 2 -A 4Repository: psychocoderHPC/alpaka3
Length of output: 299
🏁 Script executed:
# Verify that Catch2 3.5.3 includes the skip macros
rg 'catch_' cmake/buildDependencies.cmake | grep -i versionRepository: psychocoderHPC/alpaka3
Length of output: 48
🏁 Script executed:
# Double-check that SKIP is actually exported from catch_test_macros.hpp
rg 'SKIP' cmake/buildDependencies.cmake -B 5 -A 5Repository: psychocoderHPC/alpaka3
Length of output: 48
🏁 Script executed:
# Verify SKIP usage pattern is not already used elsewhere in the test suite
rg 'SKIP\s*\(' test/integ/ | head -5Repository: psychocoderHPC/alpaka3
Length of output: 48
Use SKIP() instead of silent return when device is unavailable.
The test returns without executing, which reports as passed despite never running. This masks coverage gaps in CI. Replace the return with SKIP("No device available for " << deviceSpec.getName()) to properly mark this instantiation as skipped.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/integ/computeToComputeEventHandoff/computeToComputeEventHandoff.cpp`
around lines 50 - 53, The code currently silently returns when no device is
available in the block that checks devSelector.isAvailable(), which makes the
test appear passed; replace the early return with a call to SKIP(...) so the
test is marked skipped: call SKIP(...) with a message that includes
deviceSpec.getName() (e.g., SKIP("No device available for " <<
deviceSpec.getName())), leaving the devSelector.isAvailable() check and
surrounding logic intact.
| onHost::Queue producerQueue = device.makeQueue(queueKind::blocking); | ||
| onHost::Queue consumerQueue = device.makeQueue(queueKind::blocking); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Alpaka, what are the semantics of device.makeQueue(queueKind::blocking)versusqueueKind::nonBlocking? Does enqueue on a blocking queue wait for task completion on the host, or is it still asynchronous? For an event-handoff test submitted from one host thread, would two blocking queues make the event redundant?
💡 Result:
In alpaka, device.makeQueue(queueKind::blocking) corresponds to a blocking queue (aka alpaka::Blocking), while queueKind::nonBlocking corresponds to a non-blocking queue (aka alpaka::NonBlocking). With a blocking queue, each submitted device-side command blocks the host thread until that command has finished; with a non-blocking queue, submission returns immediately and the work runs asynchronously. [1]
So, enqueue() on a blocking queue is not “still asynchronous” with respect to the host: the call only returns once the enqueued operation has completed (i.e., it behaves like an implicit per-enqueue synchronization point). This is exactly why alpaka models “sync vs async” as a queue property, rather than having separate foo()/fooAsync() APIs. [2]
For an event-handoff test (producer thread enqueues work + event; consumer thread waits on that event): if the producer uses a blocking queue, then by the time the producer thread returns from the enqueue(...) calls (including enqueue(queue, event)), the event is already complete. In that case, using the event as a handoff is typically redundant (the consumer won’t observe an “in-flight” dependency), and using two blocking queues does not restore that missing asynchrony—it just makes both sides synchronize on submission. [1]
Practical implication: if you want an event-handoff test to actually test cross-thread / cross-queue dependency, you generally need the producing side (and usually both sides) to use non-blocking queues, and then use wait(event) / “queue waits on event” for ordering. [1]
Sources:
[1] Alpaka tutorial “Lesson 34: The Queue Concept” (blocking vs non-blocking queue behavior)
[2] Alpaka docs rationale: “No Synchronous (Blocking) and Asynchronous (Non-Blocking) Function Versions” (sync/async controlled via queue type)
🏁 Script executed:
cd test/integ/computeToComputeEventHandoff && wc -l computeToComputeEventHandoff.cppRepository: psychocoderHPC/alpaka3
Length of output: 102
🏁 Script executed:
cd test/integ/computeToComputeEventHandoff && sed -n '50,100p' computeToComputeEventHandoff.cppRepository: psychocoderHPC/alpaka3
Length of output: 2211
🏁 Script executed:
cd test/integ/computeToComputeEventHandoff && sed -n '75,95p' computeToComputeEventHandoff.cppRepository: psychocoderHPC/alpaka3
Length of output: 1023
Switch to non-blocking queues or separate host threads to properly test the event-handoff.
With queueKind::blocking, producerQueue.enqueue(WriteIntermediateKernel) blocks the host until the kernel completes. By the time intermediateReady is recorded and consumerQueue.waitFor(intermediateReady) executes, the kernel has already finished, making the event-handoff ineffective. The test serializes entirely on the host and would pass identically without the event. Use queueKind::nonBlocking on the producer side (or both sides) or execute producer and consumer submissions from separate host threads to make the event-handoff actually observable and validate inter-queue synchronization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/integ/computeToComputeEventHandoff/computeToComputeEventHandoff.cpp`
around lines 57 - 58, The test uses blocking host queues which causes
producerQueue.enqueue(WriteIntermediateKernel) to block the host until
completion, so the recorded event (intermediateReady) is already signaled before
consumerQueue.waitFor(intermediateReady) runs; update the test to use
non-blocking queues by changing device.makeQueue(queueKind::blocking) to
device.makeQueue(queueKind::nonBlocking) for the producer (or both queues) or
alternatively run producerQueue.enqueue(...) and
consumerQueue.waitFor(intermediateReady) from separate host threads so the event
handoff between onHost::Queue instances is actually exercised; ensure references
to onHost::Queue, device.makeQueue, queueKind::nonBlocking,
producerQueue.enqueue(WriteIntermediateKernel), intermediateReady, and
consumerQueue.waitFor are adjusted accordingly.
1 participant
Split from local commit
6a18efcf35eb2ef9a8bca72ff3b44773dbf9984b.Base branch:
pr-add-compute-to-transfer-event-handoff-test.Summary by CodeRabbit