[MOAB-dev] commit/MOAB: 4 new changesets

[Tim Tautges]

On 06/30/2014 08:48 AM, Vijay S. Mahadevan wrote:
>> I think this name is too specific, and the allowed input is too constrained.
>> This example should be able to generate any structured mesh, not just a
>> large one.  And, there are functions that will compute the right partition
>> for you, so there shouldn't be any defaults for A/B/C.  Also, by convention
>> in structured meshes, the parameters are usually I/J/K.
>
> Agreed with everything there about the name, inputs and functionality.
> But this is just a first commit and it needs revisions based on
> comments. So its fine for now.
>
>> IMO, any changes to Interface.hpp should at least go through pull requests,
>
> There is a PR with this change although entwined within the changeset

For various reasons, I think interface changes should be isolated in their own commit.  But, I haven't been following 
PRs either, so my bad on that.

> including example addition. I think is_valid needs to be exposed since
> I've had instances when writing DMMoab when I wanted a sure fire way
> to check if something was garbage or a valid entity handle. Especially
> true for many asserts internal/external to MOAB in debug mode.
>

I don't think applications should be expected to check for valid handles, except under very specific cases (deletion 
with either non-tracking sets or handle-type tags).  That should be made very clear in the comments/documentation for 
this function.  If potential for error is that great, then put calls to is_valid under the hood on entry to functions 
passing in handles and make them either asserts or compiled under #ifndef NDEBUG.

- tim

>> I think this is fixing the symptom, not the problem.  Deleting entities
>
> I haven't looked at this in detail since I'm traveling. But Tim's
> suggestion to include this functionality directly within ParallelComm
> makes sense.
>
> Vijay
>
> On Mon, Jun 30, 2014 at 8:25 AM, Tim Tautges
> <timothy.tautges at cd-adapco.com> wrote:
>> Several things bother me about this change:
>>>
>>> --- /dev/null
>>> +++ b/examples/GenLargeMesh.cpp
>>
>>
>> I think this name is too specific, and the allowed input is too constrained.
>> This example should be able to generate any structured mesh, not just a
>> large one.  And, there are functions that will compute the right partition
>> for you, so there shouldn't be any defaults for A/B/C.  Also, by convention
>> in structured meshes, the parameters are usually I/J/K.
>>
>>
>>> diff --git a/src/moab/Interface.hpp b/src/moab/Interface.hpp
>>
>>
>> IMO, any changes to Interface.hpp should at least go through pull requests,
>> and also should be discussed on the general list first.  I'm on the fence
>> about is_valid being a function on Interface, but only because we have sets
>> that don't track owners.  The reason you included it (to allow it being
>> called by ParallelComm) IMO isn't a valid one.
>>
>>
>>
>>
>>> --- a/src/parallel/ParallelComm.cpp
>>> +++ b/src/parallel/ParallelComm.cpp
>>> @@ -8789,7 +8789,22 @@ ErrorCode
>>> ParallelComm::settle_intersection_points(Range & edges, Range & shared
>>>
>>>      return MB_SUCCESS;
>>>      // end copy
>>> +}
>>> +ErrorCode ParallelComm::correct_shared_entities()
>>> +{
>>
>>
>> I think this is fixing the symptom, not the problem.  Deleting entities
>> should check with ParallelComm in parallel cases, and there should be a
>> ParallelComm function for a collective delete entities that takes care of
>> checking the sharers.
>>
>> - tim
>>
>> --
>> Timothy J. Tautges
>> Manager, Directed Meshing, CD-adapco
>> Phone: 608-354-1459
>> timothy.tautges at cd-adapco.com

-- 
Timothy J. Tautges
Manager, Directed Meshing, CD-adapco
Phone: 608-354-1459
timothy.tautges at cd-adapco.com