AbstractAlgebra.jl
AbstractAlgebra.jl copied to clipboard
Add `Base.vec(M::MatElem)`
The code originates in https://github.com/oscar-system/Oscar.jl/blob/597398c594796cd2abbd8749977cb3f2a20eefb2/experimental/GModule/GModule.jl#L1451, but I want to use it in other parts of Oscar, that should not depend on experimental. So the cleanest solution is to add it here. Note that this code currently operates in column-wise fashion (should this be changed?)
To reduce any conflicts in Oscar, it would be easiest to include this in the next breaking release, and then remove the corresponding things in https://github.com/oscar-system/Oscar.jl/blob/597398c594796cd2abbd8749977cb3f2a20eefb2/experimental/GModule/GModule.jl#L1451.
Codecov Report
Attention: 9 lines
in your changes are missing coverage. Please review.
Comparison is base (
b4d4de5
) 86.98% compared to head (7c4c5b8
) 86.96%.
:exclamation: Current head 7c4c5b8 differs from pull request most recent head de7a624. Consider uploading reports for the commit de7a624 to get more accurate results
Files | Patch % | Lines |
---|---|---|
src/Matrix.jl | 0.00% | 9 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #1614 +/- ##
==========================================
- Coverage 86.98% 86.96% -0.03%
==========================================
Files 114 114
Lines 29686 29695 +9
==========================================
Hits 25823 25823
- Misses 3863 3872 +9
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Not clear to me if we want this.
If we don't want this, this shouldn't be in Oscar as well (pinging @fieker).
But I think we should definitely have a cheap way to convert a matrix to a vector (or 1xn / nx1 matrix)
On Mon, Feb 19, 2024 at 04:32:07AM -0800, Lars Göttgens wrote:
Not clear to me if we want this.
If we don't want this, this shouldn't be in Oscar as well (pinging @fieker). It is/ was neccessary, if we want this, it also needs to be done in Nemo for the Nemo types.
But I think we should definitely have a cheap way to convert a matrix to a vector
-- Reply to this email directly or view it on GitHub: https://github.com/Nemocas/AbstractAlgebra.jl/pull/1614#issuecomment-1952354492 You are receiving this because you were mentioned.
Message ID: @.***>
- We should not have it in this form for the same reason we don't have linear indexing: It is ambiguous.
- Is violates
A == matrix(A, nrows(A), ncols(A), vec(A))
- If it exists, it must be a view/reshape.
I am happy to rename it to _vec
in Oscar and postpone this discussion.
This seems to be controversial and I also don't see a good way to implement this efficiently without a lot of effort. So is this really worth pursuing?
If @lgoettgens still has need, perhaps we should discuss it during triage?
Iirc we resolved the problem in a different way