Extend transpose support
This extends support for transposing matrices. It builds on top of a first experiment in !1101 (closed). More specifically it implements the outcome of a discussion (!1101 (comment 89787)) with @simon.praetorius:
-
(Suitable) matrix implementations M
should have dedicated member functionM::transposed()
returning the transposed of the matrix. The result should not just be some wrapper, but a 'real' matrix. This should be implemented forFieldMatrix
,DiagonalMatrix
, andDynamicMatrix
(... andScaledIdentityMatrix
but this is in dune-istl). Transposing matrices with non-scalar entries is more delicate and should be postponed. -
The free function transpose(m)
returns a transposed version ofm
. There's two possibilities for this:- If
m
supportsm.transposed()
the function forwards to this method. - Otherwise
transpose(m)
returns a special wrapper that looks like the transposed ofm
and internally stores a copy ofm
.
- If
-
In both cases transpose(m)
does not store a reference to the passed matrix, but return an object with independent lifetime. -
The wrapper supports using the std::reference_wrapper
for opt-in capture of references. -
Cases where you explicitly want a thin wrapper storing a reference to the matrix are supported by passing a std::reference_wrapper
totranspose()
. -
By-reference-capture could also be provided by an explicit method, e.g. transposedView(m)
.
Furthermore this adds some more functionality to the wrapper:
- Add
mv
andmtv
methods. - Add conversion to a dense matrix.
- Make wrapper work for dynamically sized matrices.
The wrapper could potentially be extended by the features proposed in !962.
The major change is that transpose(m)
no longer captures by reference. This significantly improves clarity, but also imposes three potential issues. Two of them are no problem in practice, one can be fixed easily:
- The additional copy involved in
transpose(m)
should be cheap since it is currently only implemented for statically sized matrices. - The different behavior of using
auto mt = transpose(m); modifyMatrix(m); useMatrix(m);
is also no problem, because storing the result oftranspose(m)
was explicitly discouraged by the documentation. -
The wrapper implements left-multiplication by a FieldMatrix
hence one could useA*transpose(B)
with aFieldMatrix A
and aDiagonalMatrix B
. To make this work iftranspose()
no longer returns the wrapper we need to implementoperator*
forFieldMatrix
andDiagonalMatrix
as proposed in !1106 (closed).
Merge request reports
Activity
mentioned in merge request !1101 (closed)
- Resolved by Carsten Gräser
@simon.praetorius this implements the essence of our discussion. Notice that I'm very much in favor of adding more functionality to the wrapper as you suggested in !962. But that's another topic and I'd like to first get the features that are needed for dune-geometry!193 (merged).
marked the checklist item (Suitable) matrix implementations
M
should have dedicated member functionM::transposed()
returning the transposed of the matrix. The result should not just be some wrapper, but a 'real' matrix. This should be implemented forFieldMatrix
,DiagonalMatrix
, andDynamicMatrix
(... andScaledIdentityMatrix
but this is in dune-istl). Transposing matrices with non-scalar entries is more delicate and should be postponed. as completedmentioned in merge request !1075 (merged)
- Resolved by Carsten Gräser
I'm a little puzzled. I fixed the merge conflict locally and forced pushed the new version.
git status
tells me that my local branch is in sync with the one on gitlab and the newest commit is 7e5855a2. When I look at the branch in gitlab everything looks as expected, too:https://gitlab.dune-project.org/core/dune-common/-/tree/feature/extend-transpose-support
This should even be a fast forward merge. However, this MR still tells me (also after reloading several times) that there's a conflict. And the diff in the MR still shows the old version but not the current status of the branch.
Somehow the MR seems to be stuck at the old commit and isn't updating to the current version of the branch.
- Resolved by Carsten Gräser
I force pushed a new dummy commit and reverted it, but apparently gitlab recognises previous commits with the same content and doesn't show the others in the MR.
On the other hand, if you remove the DRAFT state, you should be able to merge.