-
Notifications
You must be signed in to change notification settings - Fork 201
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
Explicitly only support 1 auxiliary segment #267
Conversation
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.
Thank you! Looks good! I left some comments inline - most are pretty minor.
…aux_segment_num_rand_elements()
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.
Looks good! Thank you! I left a coupe of minor comments inline. After these are addressed, we can merge.
There is also a bigger comment about renaming. Currently we use main_segment
and main_trace
as well as aux_segment
and aux_trace
somewhat interchangeably. I think we should just use main_trace
and aux_trace
everywhere. But this renaming should probably be done in another PR.
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.
All looks good! Thank you!
Removes the half-support for multiple auxiliary segment. We now explicitly support only one.