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 _______________________________________________ Mailing list: https://launchpad.net/~dolfin Post to : dolfin@lists.launchpad.net Unsubscribe : https://launchpad.net/~dolfin More help : https://help.launchpad.net/ListHelp