vhirtham commented 6 years ago

This PR is related to issue #233. I implemented most things from the discussion. I skipped the separate interfaces for now, since I am not sure if it is necessary.

In the ElementFEM class, I specialized two functions depending on the node type:

GetDofNumbering will cause an assertion to fail, if you use it with coordinate nodes. I can also change it to an exception if you like that better.
ExtractNodeValues does basically the same for both specializations. Only difference is one naming related function call. I can put that function call into a separate function and specialize this one for maximum code re-usage, if you want.

I tried to use std::enable_if to avoid template specialization but since both functions are virtual, it is not possible or at least not trivial.

TTitscher commented 6 years ago

As I said several times in the corresponding issue, I do not like this old nuto style with virtual throw either. I think that the template approach is not suitable here. (What should TNode be, other than CoordinateNode or DofNode?) Two separate interfaces would be my preferred solution.

vhirtham commented 6 years ago

The problem here is, that separate classes do not solve the "throw" problem, cause the corresponding function is pure virtual in the elementInterace class. Even if you remove the template, you still have the "throw override" in the coordinate element. The ElementInterface itself is needed for the N and B Matrix. Solution might be to move the function to the derived classes out of the interface, but I am not sure if that works (without touching hundreds of lines of code).

TTitscher commented 6 years ago

Those separate classes may not have a common base. Or this common base has only common methods, like N, B or something. I mean, I can, again, refer to my first post in the issue, where I posted a sketch of what I would like. Maybe there is something wrong with that too, but you never really commented on it.

codecov[bot] commented 6 years ago

Codecov Report

Merging #243 into develop will decrease coverage by 5.33%. The diff coverage is 98.48%.

@@             Coverage Diff             @@
##           develop     #243      +/-   ##
===========================================
- Coverage    89.86%   84.53%   -5.34%     
===========================================
  Files          329      298      -31     
  Lines        10871    10893      +22     
===========================================
- Hits          9769     9208     -561     
- Misses        1102     1685     +583

Flag	Coverage Δ
#integrationtests	`62.22% <85.18%> (-1.58%)`	:arrow_down:
#unittests	`87.73% <94.91%> (-7.85%)`	:arrow_down:

Impacted Files	Coverage Δ
nuto/mechanics/constraints/ConstraintCompanion.h	`100% <ø> (ø)`	:arrow_up:
nuto/mechanics/iga/Nurbs.h	`83.18% <ø> (ø)`	:arrow_up:
...hanics/constitutive/damageLaws/SofteningMaterial.h	`100% <ø> (ø)`	:arrow_up:
nuto/mechanics/tools/TimeDependentProblem.h	`100% <ø> (ø)`	:arrow_up:
nuto/mechanics/mesh/MeshFem.h	`100% <ø> (ø)`	:arrow_up:
nuto/mechanics/elements/ElementIga.h	`60% <ø> (ø)`	:arrow_up:
nuto/mechanics/tools/NodalValueMerger.h	`100% <ø> (ø)`	:arrow_up:
nuto/mechanics/tools/QuasistaticSolver.h	`100% <ø> (ø)`	:arrow_up:
nuto/mechanics/cell/SimpleAssembler.h	`100% <ø> (ø)`	:arrow_up:
nuto/mechanics/tools/AdaptiveSolve.h	`100% <ø> (ø)`	:arrow_up:
... and 154 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4b14c6f...df96ede. Read the comment docs.

pmueller2 commented 6 years ago

Group "Separate geometry and dofs/interpolation" decided to continue the work started here to separate CoordinateMesh and DofMesh as a first step towards a FunctionSpaceLike thing.

The pull request will (hopefully) be reopened when we are done.

nutofem / nuto

Node coordinate #243

Codecov Report