Fix/62 view matvec and naming - #356
Open
Thiago316316 wants to merge 2 commits into
Open
Thiago316316 wants to merge 2 commits into
Thiago316316 wants to merge 2 commits into
Conversation
`Matrix` has had `Mul<Vector>` since before the views landed, but the borrowed side had no counterpart, so a product over a view had to be written out of `try_row` and `dot` at every call site. `MatrixView<ROWS, COLS> * VectorView<COLS>` now yields an owned `Vector<ROWS>`, reading both operands where they lie. It is infallible where the rest of the view surface is not: the shapes are const parameters settled at the call site rather than indices supplied at run time, so unlike `Index` there is no miss to report. The body stays total without an assertion, the way `VectorView::dot` already handles its own impossible misses, which keeps the no-panic rule intact. The property test uses the owned operator as its oracle over random matrices in both orientations. Both accumulate rows in the same order, so the two agree bit for bit and the assertion is exact. Also renames `Vector::get` / `get_mut`'s `i` parameter to `index`, and the two crate-internal unchecked accessors with it, so the rendered docs name what they take. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Mul` would have been the only infallible call on a view, and the only place the surface stopped reading as `try_*`. The failure it elided cannot occur, but a reader has to work out why before trusting that, and the operator would have been the first `core::ops` impl on a view — an invitation to ask for `Add`, `Sub`, and `Neg`, which would allocate an owned result for no gain. `MatrixView::try_mul` returns `Result<Vector<ROWS, T>, LinalgError>` like everything else here. The doc says outright that `OutOfBounds` cannot be returned, so nothing is hidden, and the module doc no longer needs a paragraph excusing one operation from its own rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
impl Mul<VectorView<', COLS, T>> for MatrixView<', ROWS, COLS, T> to match the the owned counterpart.
Vector::get / get_mut take i: usize; rename the parameter to index so it's consistent with the repository.
issues #83
Checklist
cargo test+cargo clippy --all-targetsclean locallyunwrap/expect/panicon library paths (typed errors instead)