[MOAB-dev] Discussion on return type of MOAB routines
Vijay S. Mahadevan
vijay.m at gmail.com
Fri Jan 24 10:31:00 CST 2014
If it is useful, you can use some as warnings, which can be context
dependent errors and others as fatal errors i.e., immaterial of the
context, the program will fail. e.g.,
MB_SUCCESS (=0)
MB_WARN_TAG_NOT_FOUND (>0)
MB_ERR_INDEX_OUT_OF_RANGE (<0)
If the ErrorCode is < 0, you can immediate throw the error with stack
trace but if > 0, decide based on context. Tim, I would assume the
warning codes can be treated as errors by the parent function instead
of deeper level functions in the call tree so that appropriate error
message along with stack trace can be printed and exited. This should
also specify which child function threw the error. As Jed suggested,
if you want to preserve all this context data, we could create an
ErrorCode which is a pointer to a real ErrorContext, containing a
message and other relevant info. If success, always return NULL, which
for most standard compilers should translate to 0 ? Else, look deeper
in to the ErrorContext object and print/discard needed info.
Vijay
On Fri, Jan 24, 2014 at 10:15 AM, Tim Tautges (ANL) <tautges at mcs.anl.gov> wrote:
> No, because sometimes those MB ones you list below are errors and sometimes
> not, depending on the application.
>
> - tim
>
>
> On 01/24/2014 10:13 AM, Wu, Danqing wrote:
>>
>> When designing possible error codes, it seems that PETSc only defines
>> "TRUE" errors (all with PETSC_ERR prefix)
>> #define PETSC_ERR_MIN_VALUE 54 /* should always be one less then
>> the smallest value */
>> #define PETSC_ERR_MEM 55 /* unable to allocate requested
>> memory */
>> ...
>> #define PETSC_ERR_NOT_CONVERGED 91 /* solver did not converge */
>> #define PETSC_ERR_MAX_VALUE 92 /* this is always the one more than
>> the largest error code */
>>
>> While for MOAB, we do have quite a few error codes that are not always
>> "TRUE" errors, like:
>> MB_ENTITY_NOT_FOUND
>> MB_MULTIPLE_ENTITIES_FOUND
>> MB_TAG_NOT_FOUND
>> MB_ALREADY_ALLOCATED
>> MB_VARIABLE_DATA_LENGTH
>> MB_STRUCTURED_MESH
>>
>> Would it be better to separate these non-error conditions from current
>> enum ErrorCode?
>>
>> Best
>> Danqing
>> ________________________________________
>> From: Tautges, Timothy J.
>> Sent: Friday, January 24, 2014 9:07 AM
>> To: Paul Wilson; Wu, Danqing; Vijay S. Mahadevan
>> Cc: Jed Brown; moab-dev at mcs.anl.gov
>> Subject: Re: [MOAB-dev] Discussion on return type of MOAB routines
>>
>> On 01/23/2014 10:14 PM, Paul Wilson wrote:
>>>
>>> Hi Tim,
>>>
>>> On 01/23/2014 11:05 AM, Tim Tautges (ANL) wrote:
>>>>
>>>> The trouble is then where do you leave the extra information to be
>>>> picked up if the caller needs it. We want to avoid
>>>> things like a last_error string on the instance, since that's not thread
>>>> safe.
>>>
>>> I'm imagining that you effectively repeat the same query that produced
>>> the information in the first place, but through a
>>> different interface that was setup to collect the interesting
>>> information.
>>>
>>> This may be too expensive in general, but I thought I'd throw the idea
>>> out there.
>>>
>>> Do you have a good sense of how common this would be? You offered one
>>> use case, but are there many places in MOAB that
>>> offer more information to the calling function?
>>>
>>
>> Not that many, but it's one of those things where the application
>> determines what's "normal".
>>
>> And for applications, if we pass back only an ErrorCode, plus maybe a
>> stack trace gets printed under certain conditions,
>> what about those cases where the application needs more information, e.g.
>> which handle turned out to be invalid or
>> didn't have a given tag? Then we're stuck back with a member of the Core
>> class, which we don't want because it's not
>> thread safe. Is it enough to just make that thread local?
>>
>> - tim
>>
>>
>>> Paul
>>>>
>>>>
>>>> - tim
>>>>
>>>> On 01/23/2014 07:54 AM, Paul Wilson wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> Just catching up on this thread...
>>>>>
>>>>> On 01/22/2014 03:24 PM, Wu, Danqing wrote:
>>>>>>
>>>>>> Below is the summary of the discussions on return type so far. I
>>>>>> notice that how we handle special "non-success"
>>>>>> condition (like TAG_NOT_FOUND) will affect our decision. Any decision
>>>>>> has its pros and cons. Maybe we need more
>>>>>> opinions from other people.
>>>>>>
>>>>>> As for existing ErrorCode return type, Vijay and I prefer to use it.
>>>>>> Iulian also voted to keep it simple (enum/int)
>>>>>> 1) This does not break any existing user code nor does it need to
>>>>>> change existing method signatures
>>>>>> 2) According to Jed, PETSc returns error code because it is simple and
>>>>>> portable between languages
>>>>>> 3) Benchmarking tests show that it has least overhead for function
>>>>>> return
>>>>>> 4) No need to store any context explicitly, including error messages
>>>>>> that are already printed out in the stack trace
>>>>>>
>>>>>> Tim has some other reasons to return a class object instead of a plain
>>>>>> enum type
>>>>>> 5) The overhead might be acceptable for MOAB
>>>>>> 6) For non-success conditions, we might need extra information hat can
>>>>>> only come from the lower-level functions.
>>>>>
>>>>> Since the calling function knows what it asked, couldn't this be
>>>>> addressed by a combination of:
>>>>> * return one of a rich set of int/enum return codes
>>>>> * API for querying the necessary information that is missing in a
>>>>> simple error code
>>>>>
>>>>> Cons:
>>>>> * for cases in which there is necessary information, this would be
>>>>> slower than the other options proposed because of
>>>>> another function call
>>>>>
>>>>> Pros:
>>>>> * the calling function gets to decide whether it cares about (and hence
>>>>> bothers to query) the additional information
>>>>> * there can be a richer interface for that additional information other
>>>>> than parsing a string
>>>>> * a sufficiently rich enough set of error codes, may reduce the number
>>>>> of times that a calling function cares about that
>>>>> info.
>>>>>
>>>>> Using Tim's tag_get_handle example (mismatched properties to
>>>>> tag_get_handle), if the return code was
>>>>> MOAB_TAG_EXISTS_WITH_DIFFERENT_PROPERTIES (not a great choice, but
>>>>> illustrative), then there may be many cases where the
>>>>> calling function just wants to handle this and move on, rather than
>>>>> query something like tag_get_props().
>>>>>
>>>>> Or am I missing something?
>>>>>
>>>>> Paul
>>>>>
>>>>>> Regarding 6), Vijay and I have similar opinion:
>>>>>> Is that extra information in the returned class object really
>>>>>> necessary? An upper level caller, based on some context,
>>>>>> usually knows how to handle a non-error condition. It can choose to
>>>>>> treat is as an expected error, and set it. It can
>>>>>> choose to handle it and then return success. It can also keep it
>>>>>> unhanded, and return this condition code unchanged to
>>>>>> its callers. Of course, the lowest routines, like tag_get_handle(),
>>>>>> will never set it as an error (because it does not
>>>>>> know, totally decided by higher level callers), and just returns this
>>>>>> code as is. In this case, an ErrorCode still
>>>>>> would suffice. Using an object to store any context explicitly is
>>>>>> probably not necessary here.
>>>>>>
>>>>>> BTW, according to Jed, PETSc never uses error handlers/return codes
>>>>>> for non-error conditions. It defines what should
>>>>>> happen in that case and have a flag return if the caller needs to
>>>>>> know.
>>>>>
>>>>>
>>>>>>
>>>>>> Regarding returning a class object, there are three different options:
>>>>>> 1) Use ostringstream as a member. This has more overhead according to
>>>>>> benchmarking tests. The size of the object is
>>>>>> 360 bytes.
>>>>>> 2) Use std::string as a member. This is slightly better than a). The
>>>>>> size of the object is 16 bytes.
>>>>>> 3) Make the class a POD type, like C style struct, without
>>>>>> constructors. Then char* is used as a member. A global heap
>>>>>> space should be allocated to store the string. The issue is how to
>>>>>> make this thread safe.
>>>>>> For now, it is more likely that we will use either 2) or 3)
>>>>>>
>>>>>> Best
>>>>>> Danqing
>>>>>> ________________________________________
>>>>>> From: Tautges, Timothy J.
>>>>>> Sent: Wednesday, January 22, 2014 3:36 PM
>>>>>> To: Vijay S. Mahadevan
>>>>>> Cc: Jed Brown; Wu, Danqing; moab-dev at mcs.anl.gov
>>>>>> Subject: Re: [MOAB-dev] Discussion on return type of MOAB routines
>>>>>>
>>>>>> On 01/22/2014 01:34 PM, Vijay S. Mahadevan wrote:
>>>>>>>>
>>>>>>>> It's entirely dependent on the application, same API call, same
>>>>>>>> return,
>>>>>>>> different handling, and in some cases the application is a tool
>>>>>>>> inside the
>>>>>>>> library.
>>>>>>>
>>>>>>> If the calling function knows how to handle the error based on the
>>>>>>> context i.e., fail with an error message or return with an error code
>>>>>>> to be handled by upper level routines, an ErrorCode still would
>>>>>>> suffice. Using an object to store any context explicitly is not be
>>>>>>> necessary here IMO.
>>>>>>>
>>>>>> But it's information from lower-level functions that will go missing.
>>>>>> Maybe that's not enough information to really
>>>>>> worry about, esp. if we have a stack trace, but that's the reason I'm
>>>>>> saying we need this string.
>>>>>>
>>>>>> - tim
>>>>>>
>>>>>>> If you just want the control to augment the printed error message (in
>>>>>>> fatal failure cases), use macros like SETERR that take a stream.
>>>>>>> Since
>>>>>>> the program is going to exit due to the failure, each calling routine
>>>>>>> can print its message and return appropriately without worrying about
>>>>>>> the actual cost of storage of strings etc. For successful exits,
>>>>>>> string remains null. The const char* version in my timing results
>>>>>>> will
>>>>>>> be appropriate here.
>>>>>>>
>>>>>>> Vijay
>>>>>>>
>>>>>>> On Wed, Jan 22, 2014 at 1:26 PM, Tim Tautges (ANL)
>>>>>>> <tautges at mcs.anl.gov> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 01/22/2014 01:21 PM, Jed Brown wrote:
>>>>>>>>>
>>>>>>>>> "Tim Tautges (ANL)" <tautges at mcs.anl.gov> writes:
>>>>>>>>>>
>>>>>>>>>> This depends on how often it happens. In all cases, IMO, a
>>>>>>>>>> success
>>>>>>>>>> condition should result in nothing more than a
>>>>>>>>>> POD-sized object (and I should add, no virtual table).
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> vtables are just pointers like any other in terms of performance.
>>>>>>>>>
>>>>>>>> Sure but we still don't want that extra expense.
>>>>>>>>
>>>>>>>>
>>>>>>>>>> If it's a non-success condition that is "expected" (normal
>>>>>>>>>> condition
>>>>>>>>>> that's handled by the calling code), IMO this shouldn't print
>>>>>>>>>> anything
>>>>>>>>>> by default. If it's a non-success that's also unexpected,
>>>>>>>>>> printing is
>>>>>>>>>> fine (including stack trace).
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I don't believe in expected failures. I find keeping expected and
>>>>>>>>> error
>>>>>>>>> conditions in-band leads to confusing control flow, but YMMV.
>>>>>>>>>
>>>>>>>> It's entirely dependent on the application, same API call, same
>>>>>>>> return,
>>>>>>>> different handling, and in some cases the application is a tool
>>>>>>>> inside the
>>>>>>>> library.
>>>>>>>>
>>>>>>>> - tim
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> ================================================================
>>>>>>>> "You will keep in perfect peace him whose mind is
>>>>>>>> steadfast, because he trusts in you." Isaiah 26:3
>>>>>>>>
>>>>>>>> Tim Tautges Argonne National Laboratory
>>>>>>>> (tautges at mcs.anl.gov) (telecommuting from
>>>>>>>> UW-Madison)
>>>>>>>> phone (gvoice): (608) 354-1459 1500 Engineering Dr.
>>>>>>>> fax: (608) 263-4499 Madison, WI 53706
>>>>>>>>
>>>>>> --
>>>>>> ================================================================
>>>>>> "You will keep in perfect peace him whose mind is
>>>>>> steadfast, because he trusts in you." Isaiah 26:3
>>>>>>
>>>>>> Tim Tautges Argonne National Laboratory
>>>>>> (tautges at mcs.anl.gov) (telecommuting from UW-Madison)
>>>>>> phone (gvoice): (608) 354-1459 1500 Engineering Dr.
>>>>>> fax: (608) 263-4499 Madison, WI 53706
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>> --
>> ================================================================
>> "You will keep in perfect peace him whose mind is
>> steadfast, because he trusts in you." Isaiah 26:3
>>
>> Tim Tautges Argonne National Laboratory
>> (tautges at mcs.anl.gov) (telecommuting from UW-Madison)
>> phone (gvoice): (608) 354-1459 1500 Engineering Dr.
>> fax: (608) 263-4499 Madison, WI 53706
>>
>>
>
> --
> ================================================================
> "You will keep in perfect peace him whose mind is
> steadfast, because he trusts in you." Isaiah 26:3
>
> Tim Tautges Argonne National Laboratory
> (tautges at mcs.anl.gov) (telecommuting from UW-Madison)
> phone (gvoice): (608) 354-1459 1500 Engineering Dr.
> fax: (608) 263-4499 Madison, WI 53706
>
More information about the moab-dev
mailing list