← Back to team overview

dolfin team mailing list archive

Re: interface renaming for IntersectionOperator / name suggestions

 

On Wed, Nov 09, 2011 at 03:59:28PM -0600, Andre Massing wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
>
> On 11/09/2011 03:46 PM, Anders Logg wrote:
> > On Wed, Nov 09, 2011 at 02:07:57PM -0600, Andre Massing wrote:
> >> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> >>
> >> Hi!
> >>
> >> I have implemented
> >> https://blueprints.launchpad.net/dolfin/+spec/bbtree-all-codim
> >>
> >> including some unittests, but there remain some questions.
> >>
> >> 1. The name IntersectionOperator is kind of unfortunate and
> >> cryptic. In the end it is just a wrapper for a bounding box tree
> >> so I was thinking about renaming it to something more
> >> descriptive, e.g. BoundingVolumeTree or BVTree...Any suggestions,
> >> wishes, feelings that? That brings me to another point.
> >
> > If it's a wrapper for a bounding box tree, then BoundingBoxTree
> > would be suitable, but isn't it more than that? It's an interface
> > for computing intersections between mesh entities.
>
> Yes, but that is the primary existence reason for the tree, it makes
> intersection detection feasible. But I agree, that assumes background
> knowledge from the user, so IntersectionDetector (as you suggest) is
> the better name.
>
> >
> > The constructor of the IntersectionOperator class says
> >
> > /// Create intersection detector for the mesh \em mesh.
> >
> > So perhaps IntersectionDetector could work?
>
> Yes I think so.
>
> >
> >> 2. Since a bbtree is available for all entity dimension it might
> >> be nice to have specialized, ready to use name for them
> >> (especially with the python interface in mind and similar to what
> >> we have for MeshEntity already)? For instance CellBVTree,
> >> FacetBVTree are names popping up in my mind. Are they any
> >> suggestions or preferences regarding this?
> >
> > Is this something a user will typically see? So the specialized
> > functions may not be important, but we could perhaps add
> >
> > CellIntersectionDetector
> >
> > etc.
> >
> >> 3. Johan suggested at some point to expose the distance
> >> functions available in CGAL AABBTree
> >>
> >> http://www.cgal.org/Manual/latest/doc_html/cgal_manual/AABB_tree_ref/Class_AABB_tree.html#Cross_link_anchor_1724
> >>
> >>
> >>
> which I think is a good idea.  Any objections for adding that to
> >> DOLFIN? As far as I know of intersection related operations haven
> >> 't been mentioned in the book, so an interface renaming plus some
> >> small  localized interface changes won't effect code examples in
> >> the book.
> >
> > Yes, that would be good. But I'm starting to think now that maybe
> > this should all be merged into trunk and not 1.0.x (even the fixes
> > you have made for InteresectionOperator/Detector). It's a
> > substantial piece of code and it involves renaming of classes so it
> > might introduce new bugs.
> >
> > What do you think? Is it important to get it into 1.0.0? There will
> > be many nice features in 1.1 so your code will be in good company.
> > :-)
>
> The actually change in the code to detect intersection for all
> entities  is not really a big change, basically I added some
> constructors which assures that the old code will work as before.
> Plus that I have some unittest in place checking the basic functionality.
> But your are the release manager :) If you want I can submit a merge
> request, then you can have a look at it and decide whether you think
> it is to invasive. What do you think?

As long as it works, I'm happy. :-)

--
Anders


References