-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Real world regressions for Dune and HyTeg (#22)
- Loading branch information
Showing
6 changed files
with
346 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| description: Introduces a regression into the dune-pdelab submodule due to use of shared_pointer. See https://gitlab.dune-project.org/pdelab/dune-pdelab/-/merge_requests/583 for details. | ||
| include_revisions: | ||
| revision_range: | ||
| start: 8f3dc56bda3ca663170f62903eb4b4d66e427a43 | ||
| path: FESwitch-set-store.patch | ||
| project_name: DunePerfRegression | ||
| shortname: FESwitch-set-store | ||
| tags: | ||
| - dune | ||
| - compile-time | ||
| - regression | ||
| - template | ||
| - localfunctions | ||
| - real_world | ||
| - perf_prec |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| Submodule dune-pdelab contains modified content | ||
| diff --git a/dune-pdelab/dune/pdelab/gridfunctionspace/localfunctionspace.hh b/dune-pdelab/dune/pdelab/gridfunctionspace/localfunctionspace.hh | ||
| index 594ee4e2..c82fdfff 100644 | ||
| --- a/dune-pdelab/dune/pdelab/gridfunctionspace/localfunctionspace.hh | ||
| +++ b/dune-pdelab/dune/pdelab/gridfunctionspace/localfunctionspace.hh | ||
| @@ -3,8 +3,7 @@ | ||
| #ifndef DUNE_PDELAB_GRIDFUNCTIONSPACE_LOCALFUNCTIONSPACE_HH | ||
| #define DUNE_PDELAB_GRIDFUNCTIONSPACE_LOCALFUNCTIONSPACE_HH | ||
|
|
||
| -#include <vector> | ||
| -#include <memory> | ||
| +#include<vector> | ||
|
|
||
| #include <dune/common/stdstreams.hh> | ||
| #include <dune/common/shared_ptr.hh> | ||
| @@ -100,16 +99,18 @@ namespace Dune { | ||
| void leaf(Node& node, TreePath treePath) | ||
| { | ||
| node.offset = offset; | ||
| - node.unbindFiniteElement(); | ||
| + node.pfe = nullptr; | ||
| node._in_entity_set = node.pgfs->entitySet().contains(e); | ||
| if (not node._in_entity_set) { | ||
| node.n = 0; | ||
| } else if (fast) { | ||
| node.n = node.pgfs->finiteElementMap().maxLocalSize(); | ||
| - node.bindFiniteElement(node.pgfs->finiteElementMap().find(e)); | ||
| + Node::FESwitch::setStore(node.pfe, | ||
| + node.pgfs->finiteElementMap().find(e)); | ||
| } else { | ||
| - node.bindFiniteElement(node.pgfs->finiteElementMap().find(e)); | ||
| - node.n = Node::FESwitch::basis(node.finiteElement()).size(); | ||
| + Node::FESwitch::setStore(node.pfe, | ||
| + node.pgfs->finiteElementMap().find(e)); | ||
| + node.n = Node::FESwitch::basis(*node.pfe).size(); | ||
| } | ||
| offset += node.n; | ||
| } | ||
| @@ -683,39 +684,9 @@ namespace Dune { | ||
| BaseT::bind(*this,e,fast_); | ||
| } | ||
|
|
||
| - /** | ||
| - * @brief Binds a finite element to the local space | ||
| - * If the finite element is lvalue, the caller (i.e. FEM) must guarantee | ||
| - * the lifetime of the object since we only keep a view on it. On the other | ||
| - * hand, if it is rvalue, we store it locally but we require the object to | ||
| - * be fully movable. | ||
| - * @tparam FE rvalue or lvalue type of the finite element | ||
| - * @param fe the finite element to be bound | ||
| - */ | ||
| - template<class FE> | ||
| - void bindFiniteElement(FE&& fe) { | ||
| - static_assert(std::is_same_v<std::decay_t<FE>, typename Traits::FiniteElementType>); | ||
| - if constexpr (std::is_rvalue_reference_v<FE&&>) { | ||
| - static_assert(std::is_move_constructible_v<FE>); | ||
| - static_assert(std::is_move_assignable_v<FE>); | ||
| - if (spe) | ||
| - (*spe) = std::move(fe); | ||
| - else | ||
| - spe = std::make_shared<typename Traits::FiniteElementType>(std::move(fe)); | ||
| - pfe = spe.get(); | ||
| - } else { | ||
| - pfe = &fe; | ||
| - } | ||
| - } | ||
| - | ||
| - //! Release view of the bound finite element | ||
| - void unbindFiniteElement() noexcept { | ||
| - pfe = nullptr; | ||
| - } | ||
| - | ||
| + // private: | ||
| + typename FESwitch::Store pfe; | ||
| private: | ||
| - const typename Traits::FiniteElementType * pfe; | ||
| - std::shared_ptr<typename Traits::FiniteElementType> spe; | ||
| bool _in_entity_set; | ||
| }; | ||
|
|
||
| diff --git a/dune-pdelab/dune/pdelab/ordering/directleaflocalordering.hh b/dune-pdelab/dune/pdelab/ordering/directleaflocalordering.hh | ||
| index bcf28abf..02388ebe 100644 | ||
| --- a/dune-pdelab/dune/pdelab/ordering/directleaflocalordering.hh | ||
| +++ b/dune-pdelab/dune/pdelab/ordering/directleaflocalordering.hh | ||
| @@ -209,9 +209,10 @@ namespace Dune { | ||
|
|
||
| void collect_used_geometry_types_from_cell(const typename Traits::GridView::template Codim<0>::Entity& cell) | ||
| { | ||
| - // notice that we keep the finite element alive on this scope (important if rvalue) | ||
| - const auto& fe = _fem->find(cell); | ||
| - const typename FESwitch::Coefficients& coeffs = FESwitch::coefficients(fe); | ||
| + FESwitch::setStore(_fe_store,_fem->find(cell)); | ||
| + | ||
| + const typename FESwitch::Coefficients& coeffs = | ||
| + FESwitch::coefficients(*_fe_store); | ||
|
|
||
| _max_local_size = std::max(_max_local_size,coeffs.size()); | ||
|
|
||
| @@ -249,9 +250,10 @@ namespace Dune { | ||
| if (this->_fixed_size_possible) | ||
| std::fill(_local_gt_dof_sizes.begin(),_local_gt_dof_sizes.end(),0); | ||
|
|
||
| - // notice that we keep the finite element alive on this scope (important if rvalue) | ||
| - const auto& fe = _fem->find(cell); | ||
| - const typename FESwitch::Coefficients& coeffs = FESwitch::coefficients(fe); | ||
| + FESwitch::setStore(_fe_store,_fem->find(cell)); | ||
| + | ||
| + const typename FESwitch::Coefficients& coeffs = | ||
| + FESwitch::coefficients(*_fe_store); | ||
|
|
||
| typedef typename Traits::SizeType size_type; | ||
|
|
||
| @@ -322,6 +324,7 @@ namespace Dune { | ||
| protected: | ||
|
|
||
| std::shared_ptr<const FEM> _fem; | ||
| + typename FESwitch::Store _fe_store; | ||
|
|
||
| ES _es; | ||
| bool _fixed_size; | ||
| diff --git a/dune-pdelab/dune/pdelab/ordering/leaflocalordering.hh b/dune-pdelab/dune/pdelab/ordering/leaflocalordering.hh | ||
| index 2cbc345f..88e57abc 100644 | ||
| --- a/dune-pdelab/dune/pdelab/ordering/leaflocalordering.hh | ||
| +++ b/dune-pdelab/dune/pdelab/ordering/leaflocalordering.hh | ||
| @@ -99,9 +99,10 @@ namespace Dune { | ||
|
|
||
| void collect_used_geometry_types_from_cell(const typename Traits::EntitySet::Element& cell) | ||
| { | ||
| - // notice that we keep the finite element alive on this scope (important if rvalue) | ||
| - const auto& fe = _fem->find(cell); | ||
| - const typename FESwitch::Coefficients& coeffs = FESwitch::coefficients(fe); | ||
| + FESwitch::setStore(_pfe,_fem->find(cell)); | ||
| + | ||
| + const typename FESwitch::Coefficients& coeffs = | ||
| + FESwitch::coefficients(*_pfe); | ||
|
|
||
| this->_max_local_size = std::max(this->_max_local_size,coeffs.size()); | ||
|
|
||
| @@ -124,9 +125,10 @@ namespace Dune { | ||
| if (this->_fixed_size_possible) | ||
| std::fill(gt_sizes.begin(),gt_sizes.end(),0); | ||
|
|
||
| - // notice that we keep the finite element alive on this scope (important if rvalue) | ||
| - const auto& fe = _fem->find(cell); | ||
| - const typename FESwitch::Coefficients& coeffs = FESwitch::coefficients(fe); | ||
| + FESwitch::setStore(_pfe,_fem->find(cell)); | ||
| + | ||
| + const typename FESwitch::Coefficients& coeffs = | ||
| + FESwitch::coefficients(*_pfe); | ||
|
|
||
| this->_max_local_size = std::max(this->_max_local_size,coeffs.size()); | ||
|
|
||
| @@ -163,6 +165,7 @@ namespace Dune { | ||
|
|
||
| std::shared_ptr<const FEM> _fem; | ||
| ES _es; | ||
| + typename FESwitch::Store _pfe; | ||
|
|
||
| }; | ||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| description: Reproduces a real-world regression from the dune project related to the type overload of the yasp grid. See https://gitlab.dune-project.org/core/dune-grid/-/issues/146 for details. | ||
| include_revisions: | ||
| revision_range: | ||
| start: 559e763958113c8e6c31a3569127781cadb5a535 | ||
| path: yasp-type.patch | ||
| project_name: DunePerfRegression | ||
| shortname: yasp-type | ||
| tags: | ||
| - dune | ||
| - compile-time | ||
| - regression | ||
| - template | ||
| - grid | ||
| - YaspGrid | ||
| - perf_prec | ||
| - real_world |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| Submodule dune-grid contains modified content | ||
| diff --git a/dune-grid/dune/grid/yaspgrid/yaspgridentity.hh b/dune-grid/dune/grid/yaspgrid/yaspgridentity.hh | ||
| index 702e19de8..d15fb44b2 100644 | ||
| --- a/dune-grid/dune/grid/yaspgrid/yaspgridentity.hh | ||
| +++ b/dune-grid/dune/grid/yaspgrid/yaspgridentity.hh | ||
| @@ -299,14 +299,6 @@ namespace Dune { | ||
| return Geometry(_geometry); | ||
| } | ||
|
|
||
| - /** \brief Return the name of the reference element. The type can | ||
| - be used to access the Dune::ReferenceElement. | ||
| - */ | ||
| - constexpr GeometryType type () const | ||
| - { | ||
| - return GeometryTypes::cube(Geometry::mydimension); | ||
| - } | ||
| - | ||
| /*! Return number of subentities with codimension cc. | ||
| * | ||
| * That number is (dim over (dim-codim)) times 2^codim | ||
| @@ -539,14 +531,6 @@ namespace Dune { | ||
| return Geometry( _geometry ); | ||
| } | ||
|
|
||
| - /** \brief Return the name of the reference element. The type can | ||
| - be used to access the Dune::ReferenceElement. | ||
| - */ | ||
| - constexpr GeometryType type () const | ||
| - { | ||
| - return GeometryTypes::cube(Geometry::mydimension); | ||
| - } | ||
| - | ||
| /*! Return number of subentities with codimension cc. | ||
| * | ||
| * That number is (dim over (dim-codim)) times 2^codim | ||
| @@ -877,14 +861,6 @@ namespace Dune { | ||
| return Geometry( _geometry ); | ||
| } | ||
|
|
||
| - /** \brief Return the name of the reference element. The type can | ||
| - be used to access the Dune::ReferenceElement. | ||
| - */ | ||
| - constexpr GeometryType type () const | ||
| - { | ||
| - return GeometryTypes::cube(Geometry::mydimension); | ||
| - } | ||
| - | ||
| //! return partition type attribute | ||
| PartitionType partitionType () const | ||
| { |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| description: A regression to the HyTeG project that could be introduced due to a "bad" commit by an inexperienced developer. | ||
| include_revisions: | ||
| revision_range: | ||
| start: f4711dadc3f61386e6ccdc704baa783253332db2 | ||
| path: bad-commit.patch | ||
| project_name: HyTeg | ||
| shortname: bad-commit | ||
| tags: | ||
| - HyTeg | ||
| - compile-time | ||
| - regression | ||
| - perf_prec | ||
| - real_world |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| diff --git a/src/hyteg/p1functionspace/P1Operator.hpp b/src/hyteg/p1functionspace/P1Operator.hpp | ||
| index 2e110ff2c..8b1e61cf3 100644 | ||
| --- a/src/hyteg/p1functionspace/P1Operator.hpp | ||
| +++ b/src/hyteg/p1functionspace/P1Operator.hpp | ||
| @@ -266,25 +266,11 @@ class P1Operator : public Operator< P1Function< ValueType >, P1Function< ValueTy | ||
| { | ||
| if ( storage_->hasGlobalCells() ) | ||
| { | ||
| - if ( hyteg::globalDefines::useGeneratedKernels ) | ||
| - { | ||
| - apply_face3D_generated( face, src.getFaceDataID(), dst.getFaceDataID(), level, updateType ); | ||
| - } | ||
| - else | ||
| - { | ||
| - apply_face3D( face, src.getFaceDataID(), dst.getFaceDataID(), level, updateType ); | ||
| - } | ||
| + apply_face3D( face, src.getFaceDataID(), dst.getFaceDataID(), level, updateType ); | ||
| } | ||
| else | ||
| { | ||
| - if constexpr ( hyteg::globalDefines::useGeneratedKernels ) | ||
| - { | ||
| - apply_face_generated( face, src.getFaceDataID(), dst.getFaceDataID(), level, updateType ); | ||
| - } | ||
| - else | ||
| - { | ||
| - apply_face( face, src.getFaceDataID(), dst.getFaceDataID(), level, updateType ); | ||
| - } | ||
| + apply_face( face, src.getFaceDataID(), dst.getFaceDataID(), level, updateType ); | ||
| } | ||
| } | ||
| } | ||
| @@ -306,14 +292,7 @@ class P1Operator : public Operator< P1Function< ValueType >, P1Function< ValueTy | ||
|
|
||
| if ( testFlag( cellBC, flag ) ) | ||
| { | ||
| - if constexpr ( hyteg::globalDefines::useGeneratedKernels ) | ||
| - { | ||
| - apply_cell_generated( cell, src.getCellDataID(), dst.getCellDataID(), level, updateType ); | ||
| - } | ||
| - else | ||
| - { | ||
| - apply_cell( cell, src.getCellDataID(), dst.getCellDataID(), level, updateType ); | ||
| - } | ||
| + apply_cell( cell, src.getCellDataID(), dst.getCellDataID(), level, updateType ); | ||
| } | ||
| } | ||
| } | ||
| @@ -984,25 +963,11 @@ class P1Operator : public Operator< P1Function< ValueType >, P1Function< ValueTy | ||
| { | ||
| if ( storage_->hasGlobalCells() ) | ||
| { | ||
| - if ( globalDefines::useGeneratedKernels ) | ||
| - { | ||
| - smooth_sor_face3D_generated( face, dst.getFaceDataID(), rhs.getFaceDataID(), level, relax, backwards ); | ||
| - } | ||
| - else | ||
| - { | ||
| - smooth_sor_face3D( face, dst.getFaceDataID(), rhs.getFaceDataID(), level, relax, backwards ); | ||
| - } | ||
| + smooth_sor_face3D( face, dst.getFaceDataID(), rhs.getFaceDataID(), level, relax, backwards ); | ||
| } | ||
| else | ||
| { | ||
| - if ( globalDefines::useGeneratedKernels ) | ||
| - { | ||
| - smooth_sor_face_generated( face, dst.getFaceDataID(), rhs.getFaceDataID(), level, relax, backwards ); | ||
| - } | ||
| - else | ||
| - { | ||
| - smooth_sor_face( face, dst.getFaceDataID(), rhs.getFaceDataID(), level, relax, backwards ); | ||
| - } | ||
| + smooth_sor_face( face, dst.getFaceDataID(), rhs.getFaceDataID(), level, relax, backwards ); | ||
| } | ||
| } | ||
| } | ||
| @@ -1027,14 +992,7 @@ class P1Operator : public Operator< P1Function< ValueType >, P1Function< ValueTy | ||
|
|
||
| if ( testFlag( cellBC, flag ) ) | ||
| { | ||
| - if ( globalDefines::useGeneratedKernels ) | ||
| - { | ||
| - smooth_sor_cell_generated( cell, dst.getCellDataID(), rhs.getCellDataID(), level, relax, backwards ); | ||
| - } | ||
| - else | ||
| - { | ||
| - smooth_sor_cell( cell, dst.getCellDataID(), rhs.getCellDataID(), level, relax, backwards ); | ||
| - } | ||
| + smooth_sor_cell( cell, dst.getCellDataID(), rhs.getCellDataID(), level, relax, backwards ); | ||
| } | ||
| } | ||
|
|