Collide changes - #3921
Merged
Merged
Collide changes#3921
Conversation
ritvikrao
approved these changes
May 2, 2026
ericjbohm
approved these changes
Sep 1, 2026
lvkale
added a commit
that referenced
this pull request
Sep 17, 2026
), hapiPollEvents error abort (#3938 port), Converse manual nokeep contract (#3986) * Collide changes (#3921) * Collide changes * Removed prints --------- Co-authored-by: Aditya Bhosale <adityapb1546@gmail.com> Co-authored-by: Laxmikant Kale <kale@illinois.edu> (cherry picked from commit 291d2bf) * hapiPollEvents(): detect GPU errors and abort instead of breaking out hapiEventQuery()'s return code was compared against hapiSuccess and otherwise discarded, so a genuine event error was treated exactly like "not yet complete": the poll loop broke out, the event stayed at the front of the PE's queue, and every later event behind it was never processed. The queue stalled forever with no diagnostic. Capture the return code, and when it is neither success nor hapiErrorNotReady, print the backend's error string and abort. Written with the backend-neutral spellings this line uses (hapiError_t, hapiEventQuery, hapiSuccess, hapiGetErrorString); hapiErrorNotReady was missing from hapi_portable.h and is added for both the CUDA and HIP branches. Ported from #3938 (Tom Quinn) to the reconverse line Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 92424b5) * doc/converse: document message ownership and the nokeep flag The Converse manual said only that a handler must CmiFree the message it receives. It did not say what a message sent to several destinations costs, nor that a sender can flag a message nokeep to let the runtime share one buffer among the PEs of a process. Add a "Message Ownership and the nokeep Flag" subsection to Building Messages: the default per-destination copy, the handler's ownership of its copy, what the AndFree variants transfer, and the nokeep contract (no retention past the handler's return, no modification, CmiFree exactly once on a possibly shared buffer). Name CmiSetMsgNokeep and the existing CMI_MSG_NOKEEP macro, and state where the flag takes effect on reconverse today: CmiWithinNodeBroadcast and the within-node delivery of list sends and multicasts. Also document CmiWithinNodeBroadcast, which had no entry, and cross-reference the new subsection from Writing Handler Functions, Broadcasting Messages, CmiSyncMulticast and CmiSyncListSend. Doc only; sphinx-build -W -n -b html over doc/ is warning-free. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 8b7bac7) * Converse manual: nokeep handlers free exactly once, like any other message Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 38b7777) * Converse manual: final accessor signatures (CmiSetMsgNokeep, CmiMsgIsNokeep) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit c96fd9b) * Converse manual: drop a temporal 'today' from the nokeep text Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 18a06b9) * Bump reconverse to f3ce4fd9b Moves the contrib/reconverse gitlink from 58921e9 to f3ce4fd9b (reconverse main tip, 2026-09-16), picking up: f3ce4fd Increase default PE thread stack size on macOS to prevent SIGBUS (#240) c1070b5 collectives: fan a list send out once per destination process, not once per PE (#219) (#238) 33b8c36 LCI2: apply the progress-callback reentrancy guard to issueRput (#234) f65559d Export Charm4py APIs with C linkage (#232) 32d7054 charm-ci: advisory, not gating; pull request description may pin CHARM_REF (#230) aed9554 Remove the message manager (Cmm tag-matching table): no caller anywhere (#226) c35a395 Copy-based RDMA: one acknowledgement on the initiator, no interior free (#221); runtime_modes test (#227) 0b8661d Add CI check for C compatibility of converse.h (#228) 7927857 Tests for queue, Cld, args, send variants, topology, conds, threads, handlers, abort; deque-per-level scheduler queue; topology fallback fix (#220) c4a805f tests: megarecon, the megacon Converse test bank ported to reconverse (#218) 3e29ff5 CI: build Charm++ against this reconverse and run its reconverse test tier (#224) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * reconverse-site-run: Frontier must use plain srun, not --mpi=pmi2 On Frontier, `srun --mpi=pmi2` starts each task as its own one-process reconverse job: every rank prints "Starting Reconverse with 1 process", so a two-process step is two independent single-process copies rather than one two-process job. Plain `srun` uses the cray_shasta plugin (the site default; `srun --mpi=list` offers none, cray_shasta, pmi2) and bootstraps the multi-process job correctly. Verified 2026-09-17 by in-job probes on the real binary in jobs 5495903/5495904: plain srun and --mpi=cray_shasta both print "Starting Reconverse with 2 processes", --mpi=pmi2 prints two separate one-process lines. This is the same failure mode Delta showed, fixed there by switching to --mpi=pmix in PR #3982; the frontier case had kept the pmi2 default. Consequence: the 2026-09-13 Frontier site tier (jobs 5481253, 5481302) ran with --mpi=pmi2, and all 191 of its "Starting Reconverse" lines say "with 1 process". Its multi-process and multi-node results are void and the tier needs a rerun with this commit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * reconverse-ci: clean lcrun's PMI rendezvous dir and bound the 2-process step (#3987) lcrun's LCT file PMI backend rendezvouses through one fixed directory per host ($HOME/.tmp/lct_pmi_file-<SLURM_JOBID>, so lct_pmi_file-0 with no Slurm) holding nranks/barrier/data. A process that dies between the flock-guarded nranks increment and the last rank's reset leaves the counter nonzero; the next lcrun launch then assigns a rank >= nranks, one process aborts on the assert and its peer spins forever in file_e::barrier() inside ConverseInit. The state is self-perpetuating, which is the intermittent macOS startup hang in #3987. Reported upstream as uiuc-hpc/lci#202. Until LCI isolates the directory per invocation, every step that uses lcrun removes the stale directory first, and the 2-process invocations run under timeout 900 so a bootstrap hang fails the step in 15 minutes with a clear timeout instead of consuming the job's whole budget. macOS has no GNU timeout, so the darwin job's existing brew install gains coreutils and uses gtimeout. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * ZC Post API: check the posted size on every host receive path CkPostBuffer's contract is that the posted size must be <= the source buffer's size: the runtime transfers min(posted, source) bytes and a larger posted size has no data to fill it. The device path enforces this (ckrdmadevice.C, "CkRdmaDeviceIssueRgets: posted data size is larger than source data size!"), and the host path checked it in two of its three places. The third, the CMK_ZC_BCAST_RECV_DONE_MSG branch of CkPerformRget, had none: that branch runs for a post that arrives after the node-level receive of a zerocopy broadcast already completed, so it serves the data with a memcpy out of the received buffer instead of unpacking the sender's CkNcpyBuffer and issuing an rget, and the two existing checks both sit on the unpack-and-rget path. An element that posted more than the sender sent therefore ran on with the tail of its buffer never written, silently, while a sibling element on the same node that happened to post first aborted for the same mistake. Add the check to that branch, comparing destSize against the source size recorded in the post structure, and make all three sites print the same message with both sizes and the tag, so the abort names the offending post rather than the internal function that noticed it. Nothing is guarded by CMK_RECONVERSE; classic Converse shares these paths and the same contract. The smaller-post case is unchanged. Refs #3989 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Bump reconverse to 61a69dd4a (adds #242, the RMA transfer-length fix) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * tests/zerocopy/bcast_subset_async: initialise nodegrp::evenElement The array element and group constructors set evenElement; the nodegroup constructor did not. With an uninitialised byte that is neither 0 nor 1, both the even and the odd branch of the post handler run, the node buffer is posted twice, and the second delivery of recv_zerocopy double-frees destBuffer (free(): double free detected in tcache 2, or a SIGSEGV). Found by the Frontier site tier on 2026-09-17: the same binary had passed on 2026-09-13 and started failing after the OLCF software stack update changed the heap contents at construction; the test's 1-process pass failed 20/20 on both the old and the new charm/reconverse, and 40/40 passed with this line added. Not a runtime regression. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * tests/zerocopy/bcast_subset_async: test the even/odd flag once, with one branch each Kale: the handler tested the same flag two different ways (if(evenElement) ... if(!evenElement) ...), which is what let an uninitialised byte take both branches. One if/else per class; behaviour unchanged for a properly set flag. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Revert "tests/zerocopy/bcast_subset_async: test the even/odd flag once, with one branch each" Kale: the two-way test is what exposed the uninitialised flag; leave the test's shape alone for now and keep only the initialisation fix (4d2e41c). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: AdvaitTahilyani <86711700+AdvaitTahilyani@users.noreply.github.com> Co-authored-by: Aditya Bhosale <adityapb1546@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes to the collide library to fix the checkpoint restart error seen in Quinoa.