Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

batched_span_select now guarantees element order in each span #4511

Merged
merged 12 commits into from
Aug 25, 2020
Merged

batched_span_select now guarantees element order in each span #4511

merged 12 commits into from
Aug 25, 2020

Conversation

OhadRubin
Copy link
Contributor

Solves #4498

@OhadRubin OhadRubin marked this pull request as draft July 25, 2020 11:25
@OhadRubin
Copy link
Contributor Author

OhadRubin commented Jul 25, 2020 •
edited

I'll take a look at CONTRIBUTING.md, and workshop this a little...

@dirkgr
Copy link
Member

dirkgr commented Jul 28, 2020

I am excited about fixing this issue!

@dirkgr
Copy link
Member

dirkgr commented Aug 11, 2020

Is this ready for a review yet?

@OhadRubin OhadRubin marked this pull request as ready for review August 24, 2020 21:42
@OhadRubin
Copy link
Contributor Author

Yes

dirkgr
dirkgr suggested changes Aug 25, 2020
View reviewed changes
Copy link
Member

@dirkgr dirkgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the truncated comment, this looks great. I don't know why it wasn't like this from the start :-)

allennlp/nn/util.py Show resolved Hide resolved
@OhadRubin OhadRubin requested a review from dirkgr August 25, 2020 14:52
dirkgr
dirkgr approved these changes Aug 25, 2020
View reviewed changes
Copy link
Member

@dirkgr dirkgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@dirkgr dirkgr merged commit 2dd335e into allenai:master Aug 25, 2020
@OhadRubin OhadRubin mentioned this pull request Aug 25, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dirkgr dirkgr dirkgr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@OhadRubin @dirkgr