[MOAB-dev] Discussion on return type of MOAB routines
Tim Tautges (ANL)
tautges at mcs.anl.gov
Fri Jan 24 11:02:39 CST 2014
On 01/24/2014 10:31 AM, Vijay S. Mahadevan wrote:
> 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)
>
I don't think this is much different than the current practice; you save a few conditional tests, but that's pretty minimal.
> 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.
Yes, this is as we've discussed before.
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.
But how is that different than passing back an object owning a string? Most string implementations will have the
character part dynamically allocated too, so in practice it should be almost the same. But the timing information is
very different. I wonder whether that's because the ErrorCode return is POD, which gets optimized differently than when
a class is returned?
I wonder, in the original implementation that Vijay timed, were there explicit, inlined constructor/operator= functions,
for the class with ErrorCode only and the one with string? Maybe that's what's slowing down the class version (implicit
constructor/operator= that aren't inlined)?
- tim
>
> 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
>>
--
================================================================
"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