-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Commit
Extend ind2sub and sub2ind to accept a reusable array of indices Expose rem1 and fld1, defined by analogy with mod1
- Loading branch information
There are no files selected for viewing
4 comments
on commit a559c56
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose this means we should have div1
also, although perhaps only fld1
and mod1
really make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think having the whole gamut will be useful since they make it a lot easier to deal with 1-based indexing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right – for indexing, you generally want mod1
since it always puts you in the 1:n
range even if the argument is negative. It's hard for me to think of a use case for rem1, which ends up in the 1:-1:2-n
range if the argument is negative (or should it be -1:-1:n
?). The only usage I can think of is as a performance tweak for cases where we don't think the argument will ever be negative since rem may be faster than mod, but I feel like we should handle that some other way so that people can write semantically correct code and still get good performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not that sold on rem1
: I included it only for completeness since rem
was being used in the previous implementation of the indexing operations I extended. I would agree that fld1
and mod1
are the only cases that really matter.
I would leave the type of
dims
undeclared, so people can use tuples too.Your single biggest bottleneck is the call to this function, although no one line dominates. For potentially higher performance, I would experiment with adding specialized methods:
that don't use a for-loop. I would then create
size_out
withntuple
(keeping the result as a tuple), which would allow these dimensionally-specific variants to be called when appropriate. That might also have another benefit: your lineR = Array(T, size_out...)
accounts for almost 10% of the time, and believe it or not it's almost all in thesize_out...
part (array/tuple conversions are frustratingly expensive).The upper bound on savings from these steps is a 50% speed increase, so nothing too dramatic, but might cut substantially into Matlab's minor speed advantage.