Yes exactly, that's the point, but it would get rid of a few lines of code and look simpler. Users who want to write more lines of code are free to do this:
BoundingBoxTree tree; tree.build(mesh); const std::size_t cell_index = tree.compute_first_entity_collision(x); // this may fail and give a non-context related error message Cell cell(mesh, cell_index); function.eval(values, x, cell); Compare with: AutoCell cell; // we don't even need to supply the mesh here function.eval(values, x, cell); -- Anders tis 23 juni 2015 kl 10:48 skrev Jan Blechta <[email protected]>: > On Tue, 23 Jun 2015 08:42:49 +0000 > Anders Logg <[email protected]> wrote: > > > How about something like this: > > > > 1. Require additional Cell& cell argument to Function::eval > > > > 2. Add new class AutoCell handling this for users who don't want to > > explicitly work with a BoundingBoxTree > > > > class AutoCell : public Cell > > { > > public: > > AutoCell(Mesh &mesh); > > BoundingBoxTree tree; > > } > > > > AutoCell cell(mesh); > > function.eval(values, x, cell); > > > > 3. If the mesh moves, a user can do something like: > > > > cell.invalidate() > > This is semantically equivalent to passing bounding box. AutoCell > would be just a wrapper for BoundingBoxTree, not doing anything new. > > Jan > > > > > -- > > Anders > > > > > > > > tis 23 juni 2015 kl 10:30 skrev Garth N. Wells <[email protected]>: > > > > > On 22 June 2015 at 18:00, Anders Logg <[email protected]> wrote: > > > > The challenge in moving the bounding box tree outside of the mesh > > > > is > > > that it > > > > has always been part of the mesh (it replaced an earlier data > > > > structure > > > that > > > > was there) so a user expects to be able to do > > > > > > > > v = Function(V) > > > > print v(x) > > > > > > > > > > This is fine for Expressions, but for a Function I don't think it's > > > bad for the interface to make obvious to the user that they are > > > performing a potentially expensive operation. If the user was > > > required to pass the cell, all would be fine. It would also fix the > > > issues with Function evaluations in parallel. > > > > > > > without needing to instantiate some cryptic BoundingBoxTree data > > > structure. > > > > Furthermore, a user expects that on subsequent calls v(x) is fast > > > > since > > > the > > > > tree has already been built. > > > > > > > > I don't see a way around automatic handling of building the > > > > search tree. > > > Are > > > > there some clever suggestions? > > > > > > > > > > We have a fundamental problem/flaw that MeshGeometery is mutable > > > and a Mesh owns a bounding box object. One of the two needs to give. > > > > > > Garth > > > > > > > > > > Handling this for PointSource and MultiMesh is unproblematic. > > > > (But for MultiMesh, I would rather want to move it myself.) > > > > > > > > -- > > > > Anders > > > > > > > > > > > > mån 22 juni 2015 kl 17:54 skrev Marco Morandini < > > > [email protected]>: > > > >> > > > >> >>> Besides, the mesh bounding_box_tree used to find the > > > >> >>> colliding mesh entity is cached. I fear this could be a > > > >> >>> source of "strange" results, because its use is here > > > >> >>> completely transparent to the user, who may be unaware of > > > >> >>> the need to update it. > > > >> >>> > > > >> >> > > > >> >> I really don't like magical caching. How about having > > > >> >> > > > >> >> PointSource::apply(GenericVector& b, Cell& c, double > > > >> >> magnitude); > > > >> >> > > > >> >> The user is responsible for finding the cell, and thereby also > > > >> >> responsible for handling meshes that move, etc. > > > >> >> > > > >> >> PointSource::apply(...) presently uses > > > >> >> Mesh::bounding_box_tree(), which I would like to get rid of > > > >> >> from Mesh since mesh geometry is mutable. If the search tools > > > >> >> are not cached, the user takes responsibility for managing > > > >> >> the bounding boxes. > > > >> > > > > >> > For the record, this issue is filed as > > > >> > https://bitbucket.org/fenics-project/dolfin/issue/89 > > > >> > > > > >> > > > >> Right now Mesh::bounding_box_tree() is used by > > > >> > > > >> void PointSource::apply(GenericVector& b) > > > >> void Function::eval(Array<double>& values, const Array<double>& > > > >> x) const > > > >> > > > >> and > > > >> > > > >> MultiMesh ( + MultiMeshDirichletBC ) > > > >> > > > >> It would be pretty easy to change PointSource and Function. > > > >> But I think that, for MultiMesh, I should move the bboxes there, > > > >> and leave MultiMesh::build() unchanged. > > > >> > > > >> I can go this route if there is consensus. > > > >> > > > >> Marco > > > >> > > > >> > > > >> > > > >> _______________________________________________ > > > >> fenics mailing list > > > >> [email protected] > > > >> http://fenicsproject.org/mailman/listinfo/fenics > > > _______________________________________________ > > > fenics mailing list > > > [email protected] > > > http://fenicsproject.org/mailman/listinfo/fenics > > > > > _______________________________________________ > fenics mailing list > [email protected] > http://fenicsproject.org/mailman/listinfo/fenics >
_______________________________________________ fenics mailing list [email protected] http://fenicsproject.org/mailman/listinfo/fenics
