feat: Get start index from the node#560
Conversation
|
|
||
| // Compute the flattened source indices into the source array this view points to. | ||
| // For static (non-dynamic arrays) | ||
| std::vector<ssize_t> flat_source_indices() const; |
There was a problem hiding this comment.
as a caller, how do I know whether I can call this method?
Say I have a model like this:
model = Model()
x = model.set(5, 0, 5)
y = x[:1]It is a basic indexing on a dynamic array. Nothing prevents me in principle to call that flat_source_indices(), right? I mean other than the comment that it is only for static arrays.
- removed function for flat source indices
hurdwave
changed the title
|
Release notes missing |
I wonder why #558 didn't fire. Will look into it. Edit: because it hadn't been merged yet when the PR is made. Ignore me. |
| // Infer the indices used to create the node. | ||
| std::vector<slice_or_int> infer_indices() const; | ||
|
|
||
| ssize_t start() const { return start_; } |
There was a problem hiding this comment.
In the case that the predecessor is dynamic, I think this will always be 0? As an implementation detail. Maybe better to return optional<ssize_t> and have it be empty when the predecessor is dynamic? Feels hacky but at least it's defined for all valid inputs.
There was a problem hiding this comment.
IMO the start_ value has a rather confusing definition and was only meant to be used internally in the implementation of the node. I don't think it should be exposed.
If we want an easy way to compute the first index in a basic indexing operation, we should add a utility method to the library that computes it given a set of indices and a shape. Then the BasicIndexingNode can use that utility method internally and handle the dynamic case as it needs to.
4 participants
What does this implement/fix?
Implements a function that returns the start index (the start of the view in the source array)
AI Generation Disclosure
No AI tools used.