Comments (8)
After looking at the PR (which looks great btw!) I'm a little bit uncertain about my previous statement. It might break some user's code (though should be easy to fix ...), and I'm unsure of how e.g. the from_iterator
methods are used in the wild. I'm still leaning towards panicking, but I think it would be good to have @sebcrozet's input on it before making a final decision on the desired behavior.
from nalgebra.
Good catch. We should probably make it panic, since it's likely a mistake by the user if it doesn't fit, and if it's not the user can simply call .take(n)
on their iterator before providing it to nalgebra
.
from nalgebra.
No problem!
FWIW, simply cutting off the iterator also seems like a reasonable option. This would enable things like
DVector::from_iterator(70, 0..);
Notably, the current behaviour is inconsistent at the moment: for static matrices, additional entries will be ignored, while for dynamic matrices the code will either panic or run indefinitely.
from nalgebra.
Yeah, in any case we should fix the problem of iterators running indefinitely, and make the behavior consistent 👍
from nalgebra.
Tough call. I didn’t find any occurrence of from_iter
in the standard library that would help us pick an "idiomatic" solution here. There is the (slightly different) case of Option::from_iter
which does cut off the iterator with Item = Option<T>
as soon as it yields a None
value.
In a way I prefer the option where we cut off the iterator instead of panicking, since it would avoid breaking the existing behavior for dynamic matrices (so we won’t start panicking existing code that relied on it), and it allows a couple of nice patterns like:
DVector::from_iterator(70, 0..);
as suggested by @ThomasdenH. Or even:
let mut my_iterator = ...;
let v1 = Vector3::from_iterator(&mut my_iterator); // Will get elements [0, 3) of `my_iterator`
let v2 = Vector3::from_iterator(&mut my_iterator); // Will get elements [3, 6)
let v3 = Vector3::from_iterator(&mut my_iterator); // And elements [6, 9)
from nalgebra.
Thanks for your input @sebcrozet. I agree with that for the vector case, but the matrix case is what's troubling me. I can't imagine a single case where I'd legitimately have an iterator with a different number of elements than the number of entries in a matrix. This may be a fault of my imagination, however.
In any case, I'm fine with keeping the current behavior and just cut the iterator off.
from nalgebra.
What about from_vec
and from_slice
? These currently have the same behaviour. I think supplying a vector that is too large here is less likely to be intentional. That being said, this test uses a slice that is too long.
from nalgebra.
In a way I prefer the option where we cut off the iterator instead of panicking, since it would avoid breaking the existing behavior for dynamic matrices [...]
This is not quite true, the current behaviour is
Vec
,Iterator
or slice toConst
storage:- If too small, panic
- If too large, ignore remaining entries
Vec
,Iterator
orslice
toDyn
storage:- If too small, panic
- If too large and finite, panic
- If too large and infinite, loop indefinitely
from nalgebra.
Related Issues (20)
- Request: Allow the multiplication of matrices of matrices HOT 1
- Owned MatrixView Lifetime issues HOT 2
- GAT in Allocator Trait HOT 3
- Matrix should have `sub_scalar` and `sub_scalar_mut` methods. HOT 3
- Update to syn 2.0
- Proposal: Add helper methods `is_hermitian` and `is_unitary` for the `Matrix` struct HOT 3
- Alias 'RowDVectorView<>' is missing for the result of DMatrix<T>.row() HOT 1
- Add `map_to` just like `add_to`, and `map_assign` just like `add_assign` HOT 3
- `add_to` others `*_to` methods should be a trait and return the destination reference HOT 2
- unhandled integer overflow in matrix view construction HOT 1
- Concept for reducing the verbosity of generics
- Update user guide to include matrix, stack macros
- New stack macro implementation triggers clippy::toplevel_ref_arg warning HOT 3
- Declaring const matrix/vector in nalgebra-glm?
- Quaternion.to_vector() protocol missing key specifications
- `Dyn` could have `DimName` implemented upstream
- There's a potential bug with the Mat4 inverse code. HOT 2
- Fixing assert_view_index() in matrix_view.rs
- Problem in assert_view_index function of matrix_view.rs? HOT 2
- Stuck compiling `nalgebra` on nightly HOT 5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from nalgebra.