Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add device create_sequence_table for benchmarks #10300
Add device create_sequence_table for benchmarks #10300
Changes from 20 commits
916ce00
d7f0f29
bb74cc7
0ea4f60
a25241e
dfd33f2
f9f3eec
81ac53a
6c659d4
9f5c5ba
6758095
718e269
704bb72
0ad778e
9dd9244
bda1f6c
d568d09
993c85d
bdbdf49
02ef0d2
820b417
9028a80
4f1f3e8
b31de3a
1d4d57a
581e4b8
fbd5708
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Span
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.
span can't used with initializer list. initializer list is inline, and convenient here.
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.
Then be explicit and make the parameter be
initializer_list
, notvector
. Usingvector
isn't saying what you mean.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 see why this is awkward. We can't use
span
because it can't be deduced from aninitializer_list
. However, we can't useinitializer_list
because then it prohibits passing avector
(whichspan
would allow). We need to support passing avector
, especially if we remove the behavior of cycling dtypes and require the caller to callrepeat_dtypes
to construct thatvector
before callingcreate_sequence_table
. As far as I can tell, leaving this as avector
is the only option.See also: https://quuxplusone.github.io/blog/2021/10/03/p2447-span-from-initializer-list/
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. With moving
repeate_dtypes
outside,initializer_list
parameter can't be used increate_sequence_table
. Also,span
parameter can't be used withinitializer_list
argument. implicit conversion ofinitializer_list
tovector
is only way. So, leaving this asvector
.