CEED / libCEED

CEED Library: Code for Efficient Extensible Discretizations
https://libceed.org
BSD 2-Clause "Simplified" License
197 stars 46 forks source link

CeedDelegate doesn't set functions correctly #180

Closed YohannDudouit closed 5 years ago

YohannDudouit commented 5 years ago

When I

ierr = CeedSetBackendFunction(ceed, "Ceed", ceed, "OperatorDestroy",
                               CeedOperatorDestroy_libparanumal); CeedChk(ierr);

and calling CeedElemRestrictionCreateIdentity The call to ceed->ElemRestrictionCreate calls CeedOperatorDestroy_libparanumal, removing the CeedSetBackendFunction solves the problem. I use a delegate as following:

  Ceed ceeddelegate;
  CeedInit("/gpu/occa", &ceeddelegate);
  ierr = CeedSetDelegate(ceed, &ceeddelegate);
  CeedChk(ierr);

The gdb trace:

#1  0x00007ffff7bc842c in CeedOperatorDestroy_libparanumal (op=0x0) at /home/dudouit1/Projects/libCEEDfork/libCEED/backends/libParanumal/ceed-libparanumal-operator.c:25
#2  0x00007ffff7bb6ec9 in CeedElemRestrictionCreateIdentity (ceed=0x6231a0, nelem=nelem@entry=15, elemsize=elemsize@entry=8, ndof=ndof@entry=120, ncomp=ncomp@entry=1, rstr=rstr@entry=0x7fffffffded0)
    at /home/dudouit1/Projects/libCEEDfork/libCEED/interface/ceed-elemrestriction.c:129
#3  0x0000000000400a25 in main (argc=<optimized out>, argv=<optimized out>) at /home/dudouit1/Projects/libCEEDfork/libCEED/tests/t900-libparanumal.c:19
(gdb) f 2
#2  0x00007ffff7bb6ec9 in CeedElemRestrictionCreateIdentity (ceed=0x6231a0, nelem=nelem@entry=15, elemsize=elemsize@entry=8, ndof=ndof@entry=120, ncomp=ncomp@entry=1, rstr=rstr@entry=0x7fffffffded0)
    at /home/dudouit1/Projects/libCEEDfork/libCEED/interface/ceed-elemrestriction.c:129
129   ierr = ceed->ElemRestrictionCreate(CEED_MEM_HOST, CEED_OWN_POINTER, NULL, *rstr);
(gdb) l
124   (*rstr)->elemsize = elemsize;
125   (*rstr)->ndof = ndof;
126   (*rstr)->ncomp = ncomp;
127   (*rstr)->nblk = nelem;
128   (*rstr)->blksize = 1;
129   ierr = ceed->ElemRestrictionCreate(CEED_MEM_HOST, CEED_OWN_POINTER, NULL, *rstr);
130   CeedChk(ierr);
131   return 0;
132 }
133 
jeremylt commented 5 years ago

This has nothing to do with the delegate but rather your use of the SetBackendFunction function. You didn't set OperatorDestroy correctly. See the example in the Ref backend:

  ierr = CeedSetBackendFunction(ceed, "Operator", op, "Destroy",
                                CeedOperatorDestroy_Ref); CeedChk(ierr);

From the documentation of the function

/**
  @brief Set a backend function
  @param ceed           Ceed for error handling
  @param type           Type of Ceed object to set function for
  @param[out] object    Ceed object to set function for
  @param fname          Name of function to set
  @param f              Function to set
  @return An error code: 0 - success, otherwise - failure
  @ref Advanced
**/

'Create' functions are associated with the CEED because the object does not exist prior to creation, but 'Destroy' functions are associated with the object in question (basis, operator, etc). Thus, the call should be done as shown in the Ref backend.

What happend was you overwrote the ElemRestrictionCreateIdentity slot in the Ceed struct because you called the SetBackendFunction incorrectly. I will have to think on how to get the function to raise an error when you use it incorrectly in the way you did. done

jeremylt commented 5 years ago

I modified PR #172 to prevent this sort of mistaken use of SetBackendFunction.

It is still possible to misuse this function, but it is much harder now. You would have to pass a 'type' and 'object' that do not agree. I don't know how much effort to spend on a backend developer ignoring the documentation that egregiously, but I can think about it if it would be helpful.

YohannDudouit commented 5 years ago

Ok, I got confused that the Create happens at Ceed level but Destroy at Operator level.

jeremylt commented 5 years ago

It is a easy thing to mix up given how I wrote it, so thanks for catching that.