Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Real world regressions for Dune and HyTeg #22

Merged
merged 5 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions DunePerfRegression/FESwitch-set-store.info
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
160 changes: 160 additions & 0 deletions DunePerfRegression/FESwitch-set-store.patch
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;

};

16 changes: 16 additions & 0 deletions DunePerfRegression/yasp-type.info
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
50 changes: 50 additions & 0 deletions DunePerfRegression/yasp-type.patch
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
{
13 changes: 13 additions & 0 deletions HyTeg/bad-commit.info
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
92 changes: 92 additions & 0 deletions HyTeg/bad-commit.patch
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 );
}
}

Loading