[beaminteraction]Dual Hermite shapefunctions for Lagrange Multipliers#2009
[beaminteraction]Dual Hermite shapefunctions for Lagrange Multipliers#2009dharinib98 wants to merge 17 commits into
Conversation
|
@isteinbrecher I moved the dual Hermite implementation to a separate header and addressed most of the other comments. Two points are still open: I still instantiate the rotational variants as well, since the same templated factory path is used for both positional and rotational mortar pairs. Otherwise, configurations with t_hermite_dual can still be generated and lead to linker errors. |
isteinbrecher
left a comment
There was a problem hiding this comment.
Thanks fort the changes @dharinib98 this is already looking a lot better.
Please have a look at my comments and letzt discuss if there are any questions.
Also, can you add an input file test with the dual shape functions?
aa4804d to
e4565c5
Compare
|
@isteinbrecher @maxfirmbach @mayrmt This is ready for review again! The core unit test fails in the pipeline here, because of the unit test I added for the dual hermite shape functions. However locally these tests pass for me and I not quite sure why they are problematic here. |
maxfirmbach
left a comment
There was a problem hiding this comment.
@dharinib98 Looking forward to use this feature too 😉. Looks alright to me ... I just have some minor comments and questions.
|
|
||
| namespace BeamInteraction | ||
| { | ||
| struct HermiteDual : public GeometryPair::ElementDiscretizationBase<Core::FE::CellType::line2, 2> |
There was a problem hiding this comment.
@isteinbrecher Is this what we want here? Of course using a struct forces us to a specific naming scheme, which looks a bit off in the template initialization later on. I don't have a strong opinion on this, just want to know if this would make sense for the other template parameters too in the future.
There was a problem hiding this comment.
I agree @maxfirmbach. I am not sure what the best way is, we could use an alias to define a name like t_hermite_dual after creating the struct.
|
Failing tests produce this output: |
|
|
||
| namespace BeamInteraction | ||
| { | ||
| struct HermiteDual : public GeometryPair::ElementDiscretizationBase<Core::FE::CellType::line2, 2> |
There was a problem hiding this comment.
I agree @maxfirmbach. I am not sure what the best way is, we could use an alias to define a name like t_hermite_dual after creating the struct.
|
Thanks for the changes @dharinib98!
We still need the vtu test, because we previously saw that the unit test alone is not enough. Did you verify that the vtu tests produce the correct result? One more point that we can maybe discuss in person, if we should add a runtime test that the D matrix is actually diagonal when using the dual shape functions. |
|
@isteinbrecher I have discussed the idea of testing for a diagonal D with @dharinib98 some time ago, though it is not clear to me, yet, how to achieve this in 4C. It probably must be through solve form of unit test... Most importantly, I agree that this property should be tested. |
yeah! I am not 100 percent sure either, how this can be done or rather how this can be done correctly. We can discuss about this during the week. |
b3aa746 to
1189f55
Compare
|
@isteinbrecher @dharinib98 A basic question: if we use dual shape functions and we have ensured through unit testing, that the implementation actually satisfies the biorthogonality conditions, why can't we exploit that when assembling What are your thoughts on this? |
@mayrmt The main issue with this would be (since we only deal with positional coupling and not full coupling as yet) to put the computed values into the correct location, since accessing the positional dofs and the lambda translation dofs are not easy with the current tools available. Another personal concern, might be that we would be needing a lot of extra lines of code just for this special case and might make the the current assembly structure that is in place in beaminteraction look a bit messy. |
Restore trailing newline to match main branch
b47b49d to
a39d95e
Compare
a39d95e to
87c7221
Compare
isteinbrecher
left a comment
There was a problem hiding this comment.
Thanks for addressing all the points @dharinib98 . From my side only two things are missing, the move of the diagonal check to the beaminteratcion utils and the unittest.
In theory we could do that. However, with the current code structure, we are able to combine all the different cases into one general implementation. I suspect the overhead of computing the zero entries negligible. Also, if at some point we will want to consider beams that are not fully embedded and then we will need integration again, also for dual shape functions. |
Description and Context
This is an initial working version of the implementation of dual Hermite shape functions in beaminteraction. The current implementation still requires substantial refactoring to improve code efficiency and maintainability.
Interested Parties
@mayrmt @isteinbrecher @maxfirmbach
Related Issues and Pull Requests