Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Confusing documentation on strides #1217

Open
johnmave126 opened this issue Mar 7, 2023 · 2 comments · May be fixed by #1473
Open

Confusing documentation on strides #1217

johnmave126 opened this issue Mar 7, 2023 · 2 comments · May be fixed by #1473
Labels
documentation Issues about the rustdoc-generated documentation good first issue Good first issue for newcomers.

Comments

@johnmave126
Copy link

According to RawStorage::strides

The spacing between consecutive row elements and consecutive column elements.

For example this returns (1, 5) for a row-major matrix with 5 columns.

But according to the implementation of RawStorage::linear_index:

nalgebra/src/base/storage.rs

Lines 70 to 75 in 52e6db9

#[inline]
fn linear_index(&self, irow: usize, icol: usize) -> usize {
let (rstride, cstride) = self.strides();
irow * rstride.value() + icol * cstride.value()
}

(1, 5) is a column-major matrix with 5 rows, as elements in the same column are consecutive in memory.

My personal suggested wording would be "The spacing between consecutive elements across rows and across columns.", but let's discuss. Or we could adapt the wording from NumPy.
@sebcrozet
Copy link
Member

I like the wording you suggest. And, yes, the documentation is misleading here since nalgebra does not have row-major matrices anyway.

@sebcrozet sebcrozet added bug good first issue Good first issue for newcomers. documentation Issues about the rustdoc-generated documentation and removed bug labels Apr 7, 2023
@RaulWCosta
Copy link

RaulWCosta commented Aug 5, 2023 •
edited
Loading

Hey! If this is still available, I'd love to contribute with a PR :)

@wowinter13 wowinter13 linked a pull request Jan 14, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
documentation Issues about the rustdoc-generated documentation good first issue Good first issue for newcomers.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

docs: fix documentation on strides wowinter13/nalgebra
3 participants
@johnmave126 @sebcrozet @RaulWCosta