Closed ritvikrao closed 4 months ago
I did some additional looking into this. The CkCallWhenIdle function was initially implemented with a "double time" parameter, seemingly to match the arguments of the other conditional callback functions (even though the time didn't do anything). A later PR (https://github.com/charmplusplus/charm/commit/245542a9986538c2327da5a0c3e56f4b8913133e#diff-4fde7c6629d416c117d4bae763d02a64f272275fb021e1c4ab0a9a526c3062aa) apparently intended to get rid of that parameter as part of a set of changes to avoid timers in conditional callbacks. It even adds a change in xi-Entry.C to throw an error if any arguments are passed to the target of a CkCallWhenIdle callback. However, the PR never made any change to the translation of CkCallWhenIdle, so the "time" attribute is still added to the function (https://github.com/charmplusplus/charm/blob/92fa36ab0a9728298278e713b7e61eb6b2d4af42/src/xlat-i/xi-Entry.C#L2315).
you mean "PR never made any change to the translation of whenidle entry method attribute". This should be fixed , since its easy. However: I wonder if “whenidle” attribute provides any abstraction benefit. since you need registration anyway (so it appears.. just adding the attribute is not enough, at least according to the example. Maybe the intention was no registration should be needed). Users can just use the underlying converse functionality to call a function when idle. The only utility of the entry method attribute I see is that it can be repeately called based on the return value.. easy enough to do by registering the callback everytime if needed. and avoids the oddness of an entry method with a bool return value. @jszaday ?
You can see the motivations for [whenidle] here: https://github.com/charmplusplus/charm/issues/2042
The example should be built and run in CI testing to ensure it doesn't break going forward. Also [local] and [sync] entry methods already return values, so I don't think whenidle's return value is any weirder than those.
Thanks. That clarfies the issue. I do think the "ugliness" is only slightly mitigated, but the benefit of traceability (because idleProgress is an entry method now) is quite useful.
When running "make" in examples/charm++/whenidle, I get the following error:
This is with the ofi-cxi x86 build on Delta, but I don't think the build is the cause of this problem. The first of the 2 errors here goes away by removing the 0 in the CkCallWhenIdle function, but the second error remains.