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.

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

Implement Default for all matrix types in nalgebra and nalgebra-sparse #1204

Open
Andlon opened this issue Feb 9, 2023 · 4 comments
Open
Labels
good first issue Good first issue for newcomers.

Comments

@Andlon
Copy link
Collaborator

Andlon commented Feb 9, 2023

Currently, VecStorage has no Default impl, and CsrMatrix, CscMatrix etc. have no default impls.

@Andlon Andlon added the good first issue Good first issue for newcomers. label Feb 9, 2023
SoutrikBandyopadhyay added a commit to SoutrikBandyopadhyay/nalgebra that referenced this issue Feb 13, 2023
This commit solves dimforge#1204 and implements the Default trait for CsMatrix,
CscMatrix, SparsityPattern, VecStorage.
@AnbyKatz
Copy link

AnbyKatz commented Jun 9, 2023

I wouldn't mind giving this one a try. Would it be preferred that they generate identity matrices or 0 matrices as defaults?

@acxz
Copy link

acxz commented Sep 25, 2023

I just picked up this library and came across this issue as well.
Just running the following line:

let x = nalgebra::SVector::<f64, 37>::default();

fails with the following:

error[E0599]: the function or associated item `default` exists for struct `Matrix<f64, Const<37>, Const<1>, ArrayStorage<f64, 37, 1>>`, but its trait bounds were not satisfied
   --> src\bin\main.rs:136:51
    |
136 |     let x = nalgebra::SVector::<f64, 37>::default();
    |                                                   ^^^^^^^ function or associated item cannot be called due to unsatisfied trait bounds
    |
   ::: C:\Users\acxz\.cargo\registry\src\index.crates.io-6f17d22bba15001f\nalgebra-0.32.3\src\base\array_storage.rs:46:1
    |
46  | pub struct ArrayStorage<T, const R: usize, const C: usize>(pub [[T; R]; C]);
    | ---------------------------------------------------------- doesn't satisfy `ArrayStorage<f64, 37, 1>: Default`
    |
   ::: C:\Users\acxz\.cargo\registry\src\index.crates.io-6f17d22bba15001f\nalgebra-0.32.3\src\base\matrix.rs:175:1
    |
175 | pub struct Matrix<T, R, C, S> {
    | ----------------------------- doesn't satisfy `_: Default`
    |
    = note: the following trait bounds were not satisfied:
            `ArrayStorage<f64, 37, 1>: Default`
            which is required by `Matrix<f64, Const<37>, Const<1>, ArrayStorage<f64, 37, 1>>: Default`   

Values up to 32 do work however, looks like for static vectors, default isn't implemented for sizes up to 64.

Would implementing this be feasible and part of the roadplan?

Edit: going through the code I see that values up to 127 are included:

from_to_typenum!(
U0, 0; /*U1,1;*/ U2, 2; U3, 3; U4, 4; U5, 5; U6, 6; U7, 7; U8, 8; U9, 9; U10, 10; U11, 11; U12, 12; U13, 13; U14, 14; U15, 15; U16, 16; U17, 17; U18, 18;
U19, 19; U20, 20; U21, 21; U22, 22; U23, 23; U24, 24; U25, 25; U26, 26; U27, 27; U28, 28; U29, 29; U30, 30; U31, 31; U32, 32; U33, 33; U34, 34; U35, 35; U36, 36; U37, 37;
U38, 38; U39, 39; U40, 40; U41, 41; U42, 42; U43, 43; U44, 44; U45, 45; U46, 46; U47, 47; U48, 48; U49, 49; U50, 50; U51, 51; U52, 52; U53, 53; U54, 54; U55, 55; U56, 56;
U57, 57; U58, 58; U59, 59; U60, 60; U61, 61; U62, 62; U63, 63; U64, 64; U65, 65; U66, 66; U67, 67; U68, 68; U69, 69; U70, 70; U71, 71; U72, 72; U73, 73; U74, 74; U75, 75;
U76, 76; U77, 77; U78, 78; U79, 79; U80, 80; U81, 81; U82, 82; U83, 83; U84, 84; U85, 85; U86, 86; U87, 87; U88, 88; U89, 89; U90, 90; U91, 91; U92, 92; U93, 93; U94, 94;
U95, 95; U96, 96; U97, 97; U98, 98; U99, 99; U100, 100; U101, 101; U102, 102; U103, 103; U104, 104; U105, 105; U106, 106; U107, 107; U108, 108; U109, 109; U110, 110;
U111, 111; U112, 112; U113, 113; U114, 114; U115, 115; U116, 116; U117, 117; U118, 118; U119, 119; U120, 120; U121, 121; U122, 122; U123, 123; U124, 124; U125, 125; U126, 126;
U127, 127
);

Is there a reason or has it been simply overlooked that sizes above 32 do not implement the default trait?

@Tamchuk
Copy link

Tamchuk commented Jan 29, 2025

@arscisca is this still open? #1312 supposedly solves this...

@arscisca
Copy link
Contributor

Hello! I haven't checked back on this since the pull request, but yes I implemented the Defaults for the mentioned structs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good first issue for newcomers.
Projects
None yet
Development

No branches or pull requests

5 participants