Bug #804
ZeroMat and IdentityMat should produce a matrix not a ConstMatrixView
Description
I think that ZeroMat and IdentityMat should produce a result of type matrix and not one of type ConstMatrixView. The reason I think this is that MatrixView and ConstMatrixView are supposed to offer "matrix views" into a datastructure owned by another object, where as a matrix is an independent object (apart from the rings to which the elements belong).
Discuss!
Related issues
History
#1
Updated by John Abbott over 8 years ago
Updated by - Status changed from New to In Progress
- % Done changed from 0 to 20
Rather to my surprise I have failed to create an example where ZeroMat (or IdentityMat) misbehaves -- no idea why! Perhaps something to do with inheritance and virtual destructors (but isn't there a risk of slicing?)
My understanding is that a MatrixView or a ConstMatrixView does not own the underlying object, yet a ZeroMat and an IdentityMat are independent objects, so for cleaniless they must be owned. If the pseudo-ctors returned objects of type matrix then the matrix will be the owner. If we change the return type to matrix then the code should perhaps be moved out of MatrixView.C.
I tried to write program which exhibits the lack of tidying up, but have not yet succeeded (perhaps because I have not yet fully understood what the cause is).
#2
Updated by John Abbott over 8 years ago
- Assignee set to John Abbott
- % Done changed from 20 to 30
I now think that the old code was "safe" (i.e. no risk of slicing etc). However it was definitely confusing that the type was a "view" (into what???)
I now have a new impl which creates "proper, independent objects" derived from ConstMatrixBase. I do think that this is a cleaner impl than the previous one, and is also easier to comprehend.
#3
Updated by John Abbott over 8 years ago
I am unsure where the put the new defns of ZeroMat and IdentityMat.
They should not be in MatrixView.C since they are no longer views. While they do have something in common (just constness?) with the matrices for orderings (in MatrixForOrdering.C), it would be strange to put them in a file with that name.
For the time being they are in a new file called MatrixBasic.C, but I not like that name.
Do you have any ideas for the file name?
One possibility is ZeroMat.C and IdentityMat.C, but it is a shame that these names do not start with Mat.... Perhap I could use MatZero.C and MatIdentity.C (but then why do the fns have different names?)
Ideas? Suggestions?
#4
Updated by John Abbott over 8 years ago
I have another doubt/question.
The pseudo-ctors for matrices for orderings do not accept a ring as an arg; the matrix is always over ZZ (or perhaps QQ??).
Should there also be the possibility to create a ZeroMat and an IdentityMat without specifying the ring? The idea is that the default ring is the same as that for the matrices for orderings.
Comments? Criticisms? Ideas?
#5
Updated by John Abbott over 8 years ago
- % Done changed from 30 to 40
Just spoken to Anna (via Skype) about the idea of non specifying the ring.
It seems safer for IdentityMat and ZeroMat to require the ring explicitly. If the default ring were ZZ, we think that in most cases the the ring needed would be different, so would have to be specified anyway.
#6
Updated by John Abbott over 8 years ago
- Status changed from In Progress to Feedback
- % Done changed from 40 to 90
IdentityMat and ZeroMat are now in matrix.H -- Anna's suggestion.
The args are now MachineInt instead of just long.
Documentation updated. All checked in.
#7
Updated by John Abbott about 8 years ago
- Status changed from Feedback to Closed
- % Done changed from 90 to 100
- Estimated time set to 4.30 h