-
Notifications
You must be signed in to change notification settings - Fork 120
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
Fix size hint and values read with sparse iterator #411
Commits on Jan 8, 2024
-
Add test for size hint on sparse accessor
Currently this fails due to a faulty size_hint() implementation in SparseIter. For the test asset it claims that 3 items will be read from the iterator, when the correct answer is 14. 3 is the number of positions overriden by the sparse values, while the total number of positions is 14. The bug itself will be fixed in an upcoming change.
Configuration menu - View commit details
-
Copy full SHA for edda147 - Browse repository at this point
Copy the full SHA edda147View commit details -
Add test for values read in sparse accessor
The `collect()` in this test currently triggers a `attempt to subtract with overflow` panic inside `SparseIter::size_hint()`. This will be fixed in a following commmit.
Configuration menu - View commit details
-
Copy full SHA for d09cbcb - Browse repository at this point
Copy the full SHA d09cbcbView commit details -
Fix bug in size hint of sparse iterator
The fix is to delegate size hint to `self.base` as it knows how many values are left to consume. Note that `base` is only set if `accessor.bufferView` is set. If it isn't then the sparse iterator actually has no clue on how many items are left. But it at least knows that there are at least `self.values` items left. This issue will be improved in a follow up PR. The problem with the old code was that when `self.values.len()` is less than `self.counter`, a panic was triggered: `attempt to subtract with overflow`. To understand why we need to understand what the `values` and `counter` variables. `self.values` is an iterator over the sparse values that overwrites values from the base accessor. Each call to `SparseIter::next()` will consume one item and `self.values.len()` consequently decrease by 1. `self.counter` holds how many items have been consumed or seen another way the number of successfull calls to `SparseIter::next()`. It starts at 0 and increase until all values in the base accessor has been consumed. Now you hopefully see that the old implementation was just plain wrong.
Configuration menu - View commit details
-
Copy full SHA for cae9a76 - Browse repository at this point
Copy the full SHA cae9a76View commit details -
Add test for size hint on sparse accessor without buffer view
For this test case, the iterator gives a size hint of 1 output value. The correct answer is found in `accessor.count`, which is `2`.
Configuration menu - View commit details
-
Copy full SHA for f57d145 - Browse repository at this point
Copy the full SHA f57d145View commit details -
Add test for values read in sparse accessor without buffer view
This test triggers an infinite loop, as `next()` in `SparseIter` does not have any end condition whenever the base buffer view is not set in a sparse accessor.
Configuration menu - View commit details
-
Copy full SHA for 263af2a - Browse repository at this point
Copy the full SHA 263af2aView commit details -
Enable reading sparse accessor without buffer view
When `accessor.bufferView` is unset the sparse iterator should use T::zero() for `accessor.count` items as base. Then possibly overwriting them with the values in specified by the accessor.sparse section. The sparse iterator is passed `accessor.count` here, so that it knows when it should stop generating items. Before it continued until the end of times.
Configuration menu - View commit details
-
Copy full SHA for a195033 - Browse repository at this point
Copy the full SHA a195033View commit details
Commits on Jan 9, 2024
-
Configuration menu - View commit details
-
Copy full SHA for ca8655d - Browse repository at this point
Copy the full SHA ca8655dView commit details