charmplusplus/charm

whenidle example does not seem to work

ritvikrao opened this issue · 5 comments

When running "make" in examples/charm++/whenidle, I get the following error:

../../../bin/charmc  -c idlework.ci idlework.C
idlework.C: In member function 'void Test::registerIdleWork()':
idlework.C:15:46: error: no matching function for call to 'CkIndex_Test::idleProgress(int)'
   15 |     CkCallWhenIdle(CkIndex_Test::idleProgress(0), this);
      |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
In file included from idlework.h:2,
                 from idlework.C:2:
idlework.decl.h:176:16: note: candidate: 'static int CkIndex_Test::idleProgress()'
  176 |     static int idleProgress() { return idx_idleProgress_void(); }
      |                ^~~~~~~~~~~~
idlework.decl.h:176:16: note:   candidate expects 0 arguments, 1 provided
In file included from idlework.C:28:
idlework.def.h: In static member function 'static void CkIndex_Test::_call_idleProgress_void(void*, double)':
idlework.def.h:299:36: error: no matching function for call to 'Test::idleProgress(double&)'
  299 |   bool res = impl_obj->idleProgress(time);
      |              ~~~~~~~~~~~~~~~~~~~~~~^~~~~~
idlework.C:18:6: note: candidate: 'bool Test::idleProgress()'
   18 | bool Test::idleProgress() {
      |      ^~~~
idlework.C:18:6: note:   candidate expects 0 arguments, 1 provided
Fatal Error by charmc in directory /u/rao1/charm/ofi-linux-x86_64-cxi-slurmpmi2cray-smp/examples/charm++/whenidle
   Command g++ -DCMK_GFORTRAN -D_REENTRANT -I/u/rao1/charm/ofi-linux-x86_64-cxi-slurmpmi2cray-smp/include -I/opt/cray/pe/pmi/6.1.13/include -I/usr/include/slurm/ -I/opt/cray/libfabric/1.15.2.0/include -I/usr/include/ -I./proc_management/ -I./proc_management/simple_pmi/ -I./proc_management/ -I/opt/cray/libfabric/1.15.2.0/include -D__CHARMC__=1 -U_FORTIFY_SOURCE -fno-stack-protector -fno-lifetime-dse -c idlework.C -o idlework.o returned error code 1
charmc exiting...
make: *** [Makefile:10: idlework.o] Error 1

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.

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 (245542a#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 (

str << " bool res = impl_obj->" << name << "(time);\n";
).

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: #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.

fixed by #3820