itaps-parallel Proposal for Requiring Save/Restore of Sets and Tags

[Carl Ollivier-Gooch]

On 10/21/2010 11:45 AM, Jason Kraftcheck wrote:
> On 10/21/2010 01:30 PM, Carl Ollivier-Gooch wrote:

>> The -real- issue is that there are internal checks that should be done
>> to help API users with debugging.  This is why we return errors.  Yes,
>> this is something the compliance tests check for, but that's not why
>> they're there.
>>
>
> We return errors when things fail.  That is an obviously well accepted
> programming practice.  Failure to return an error flag of some kind when an
> operation fails is a bug (regardless of whether or not one is in a "release"
> mode.)  That is distinct from helping users with debugging, which should be
> an implementation issue.

But those error returns are precisely the debugging aid to which I 
refer.   If the iMesh spec says that an error code should be returned 
when someone tries to iterate over all faces of topology prism, then 
users should get that error code when they make that silly mistake. Same 
goes for other behavior the spec says is erroneous.  This is different 
than returning failures for internal errors, which, as you say, an 
implementation should also do.

>> And yes, the implementation that's used is practice should be the one
>> that's tested.  What I'm saying is -specifically- that the -test- can be
>> disabled in production mode.  By this time, presumably the API user has
>> code that is functioning properly and all those tests will pass anyway,
>> so they're a waste of time.  The only exception, I suppose, would be
>> differences in behavior for different input, but those strike me as
>> likely to be rare.
>>
>
> If the implementation is unusable in "testing" mode because it is doing some
> O(n^2) checking operation then have we really helped the developer?

No.  But I don't know that mandate any error checking that's that 
expensive, at least with good implementation.

>> I don't know about you, but when I use #ifndef NDEBUG, I -only- put
>> things in those blocks that are pure tests with no side effects.  The
>> code that produces useful results (i.e., the production code) goes
>> outside these.
>
> Is there something you're trying to imply? :)

Not at all, but if you think the shoe fits... :-)

>>   The debug-mode code does a lot of work on the side
>> without affecting any output.  That's the programming style I think
>> makes sense here, too:  write production code and decorate it with the
>> necessary checks, some or all of which will be disabled in production
>> code, without affecting what the production code does.
>>
>
> That's a great coding practice.  However, I a) don't think it is appropriate
> for this particular case and b) don't see why you consider it part of the
> iMesh API rather than an implementation choice that we made.

Checks for conditions that the iMesh spec says should return an error 
don't fall into the category of implementation choices, IMO.  You seem 
to be saying the same thing when you talk about appropriateness for this 
case.  Recognizing that some checks may be expensive (hopefully not 
n^2!), we've currently given implementations the option of not checking 
everything in production code.  This may be something we need to 
consider, or at least make a statement like "all tests that are constant 
time must always be done".  This is probably most of them, actually...

>> So no, I don't think this is the beginning of a slippery slope.  And I
>> certainly disagree that there's anything involved that's focused
>> specifically at compliance tests.
>>
>
> So if users never enable our "strict conformance" mode and we don't
> otherwise test for this does it really matter that the compliance test
> checked this and that our implementation passed?

I suppose not in a practical sense.  But in this case, users who are 
developing against your implementation may be doing things that aren't 
supported by other implementations.  This is something we should avoid 
wherever we can.

Coming back to the present case, we seem to have two choices:

1.  Keep the present semantics and (to keep interoperability) mandate 
the check.

2.  Change the current semantics, rendering the check irrelevant.  If 
we're going to do this, we face the issue of supporting both directed 
and undirected graphs, presumably.  Here the interoperability problem 
would be limited to a transition period when we have some 
implementations that don't fully support undirected and/or cyclic 
graphs.  I think I could support cyclic directed graphs by removing a 
check.  Undirected graphs could be really easy or could require 
significant re-write; I'm not sure without dissecting the code.

Carl

-- 
------------------------------------------------------------------------
Dr. Carl Ollivier-Gooch, P.Eng.                   Voice: +1-604-822-1854
Professor                                           Fax: +1-604-822-2403
Department of Mechanical Engineering             email: cfog at mech.ubc.ca
University of British Columbia              http://www.mech.ubc.ca/~cfog
Vancouver, BC  V6T 1Z4                  http://tetra.mech.ubc.ca/ANSLab/
------------------------------------------------------------------------