Feature/topology to geometry type
This is part of !159 (merged) containing the parts that can be directly merged into master before a release, since they shouldn't break code
Disucssion:
-
@andreas.dedner: suggestion to keep the
addConeetc methods on theGeometryType - @oliver.sander: I suggested that change for purely political reasons: Adding public methods to GeometryType is an interface change that needs discussion, voting, ... stuff that takes time. Adding code to the Impl namespace is much easier.
- @oliver.sander: I don't mind public methods per se. It's just that we have to give them some extra attention, because they are so difficult to get rid of once they are there.
- @carsten.graeser: I agree with @andreas.dedner that addCone() and addTensor() are nice additions to the public interface. popHighest() is a different thing, because it very much assumes that one is aware of the implementation details of the GeometryType, namely the TopologyId. But this could be improved by writing more documentation and finding a better name. In the short run keeping this in Impl:: avoids any problems.
bool GeometryType::isPrismatic(step=dim-1) // undefined for step < 0 or step > dim-1
bool GeometryType::isConical(step=dim-1) // undefined for step < 0 or step > dim-1
constexpr GeometryTypes::GeometryTypes::prismaticExtension(gt)
constexpr GeometryTypes::GeometryTypes::conicalExtension(gt)
constexpr Impl::GeometryTypes::getBase()Merge request reports
Activity
@henrik.stolzmann you need to do a 'git push' in your local repo to add the
toIdchanges.Made
addConeandaddTensormemberfunctions and renamedpopHighesttogetBase.Edited by Henrik Stolzmann
356 356 return static_cast<Id>(id); 357 357 } 358 358 359 constexpr Id toId() const I neither want to open a third MR nor mess up the existing WIP ones. So here's my
359 constexpr Id toId() const 359 /** 360 * \brief Create an Id representation of this GeometryType. 361 * 362 * The returned Id encapsulates the whole information of this 363 * GeometryType into an enum suitable for being used as template 364 * parameter. The GeometryType can be reconstructed from the Id 365 * using GeometryType{id}. 366 */ 367 constexpr Id toId() const Maybe one could add that the function was only introduced to support older gcc versions( <10.20)? If the support for these versions is droppped you can easily remove the function again.
359 constexpr Id toId() const 359 /** 360 * \brief Create an Id representation of this GeometryType. 361 * 362 * The returned Id encapsulates the whole information of this 363 * GeometryType into an enum suitable for being used as template 364 * parameter. The GeometryType can be reconstructed from the Id 365 * using GeometryType{id}. 366 * 367 * This function was mainly introduced to support older GCC versions (<10.2). 368 * There the implicit conversion from GeometryType to Id failed if a pure r-value 369 * template argument based on a static class member was used. 370 * (See dune/geometry/test/test-geometrytype-id.cc) 371 */ 372 constexpr Id toId() const Edited by Henrik StolzmannI think having a named function is reasonable anyway.
Edited by Carsten Gräser
@henrik.stolzmann did you open a companion MR in dune-localfunction? I thought that I remembered that you did but it didn't get around to checking it out locally and it might have been lost on the main repo.
I already opened a new MR.
I was looking for a branch on the main repo. The problem with external branches is that they are not picked up by the CI in other modules. So I don't see any easy way to test our feature branch in dune-fem with the corresponding branch in dune-localfunctions (the dune-geometry one is now on the main repo so that is picked up).
649 667 650 668 } 651 669 670 namespace Impl 671 { 672 673 /** \brief Return true if entity was constructed with a conical product in the last step */ 674 constexpr bool isCone(const GeometryType& gt) { 675 return ! gt.isNone() && (((gt.id() & ~1) & (1u << (gt.dim()-1))) == 0); 676 } 677 /** \brief Return true if entity was constructed with a tensor product in the last step */ 678 constexpr bool isTensor(const GeometryType& gt) { We still haven't come up with a voting system, right? I'll set up an issue with the suggested additional methods and ask for objection. Would that work for you @Oliver Sandermailto:oliver.sander@tu-dresden.de
changed this line in version 5 of the diff
695 static constexpr GeometryType geometry = geometryId; 696 template< class... Args > 697 static auto apply ( unsigned int topologyId, Args &&... args ) 698 { 699 if( topologyId & 1 ) 700 return IfGeometryType< Operation, dim-1, geometry.addTensor().toId() >::apply( topologyId >> 1, std::forward< Args >( args )... ); 701 else 702 return IfGeometryType< Operation, dim-1, geometry.addCone().toId() >::apply( topologyId >> 1, std::forward< Args >( args )... ); 703 } 704 }; 652 705 706 template< template< GeometryType::Id > class Operation, GeometryType::Id geometryId > 707 struct IfGeometryType< Operation, 0, geometryId> 708 { 709 template< class... Args > 710 static auto apply ( unsigned int topologyId, Args &&... args ) changed this line in version 4 of the diff
27 constexpr Dune::GeometryType gt2b = foo2.gt; 28 29 static_assert(gt2a == gt2b, "The two geometry types have to compare equal"); 17 30 18 Foo<gt1> foo; 31 Foo<gt2b.addTensor()> foo3; 19 32 20 constexpr Dune::GeometryType gt2 = foo.gt; 33 constexpr Dune::GeometryType gt3 = foo3.gt; 21 34 22 static_assert(gt1 == gt2, "The two geometry types have to compare equal"); 35 static_assert(gt3 == Dune::GeometryTypes::prism, "The two geometry types have to compare equal"); 23 36 37 if (foo2.apply() != foo3.gt.id()) 38 { 39 std::cout << "Error in topology ids\n"; changed this line in version 4 of the diff
676 } 677 /** \brief Return true if entity was constructed with a tensor product in the last step */ 678 constexpr bool isTensor(const GeometryType& gt) { 679 return ! gt.isNone() && (( (gt.id() | 1) & (1u << (gt.dim()-1))) != 0); 680 } 681 682 /** \brief Removes the bit for the highest dimension and returns the lower-dimensional GeometryType */ 683 inline constexpr GeometryType getBase(const GeometryType& gt) { 684 return GeometryType(gt.id() & ((1 << (gt.dim()-1))-1), gt.dim()-1, gt.isNone()); 685 } 686 687 688 // IfGeometryType 689 // ---------- 690 691 //template< template< class > class Operation, int dim, class Topology = Point > changed this line in version 4 of the diff
16 16 These are optimal rules that include only one endpoint of the integration interval 17 17 (either left or right) and integrate polynomials of order 2n - 2 exactly. 18 18 19 - GeometryType has two new methodes: `addCone` and `addTensor`. changed this line in version 5 of the diff
This adds
addConeandaddTensorto the public interface which is IMHO a good idea. I'm just wondering ifaddTensoris a good name. I don't see in which sense this can be regarded a tensor product construction. On the one hand, there's no vector spaces. On the other hand this also isn't true if you consider the spaces spanned by the polyhedra. In terms of the sets (and spanned vector spaces) one does a simple cartesian product.Following the naming scheme of
addConeusingaddPrismis far more natural. Semantically one may also argue, that this does not add a cone/prism to the existing GT, but makes a cone/prism. Butmake(Cone|Prism)may mistakenly be understood to be in place operations.
addProductwould be another alternative which is still and improvement, because the construction is based on a cartesian product and not a tensor product.I agree with Carsten tat the naming is not optimal. Also the
AddConedoes not really phrase the topological operation.I discussed tour constructions a while ago with a colleague from pure math and he pointed me to this paper: https://arxiv.org/abs/1603.03585
The authors there discribe the construtions we use in the following way
- prisms are the cartesian product of a segment with a polygon
- pyramids are the join product of a point with a polygon
So both are products. How about
addCartesianProductandaddJointProduct?
