Skip to content
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

ENH: Select top and bottom tiles on the fly #388

Merged
merged 194 commits into from
Jun 7, 2022

Conversation

kenza-bouzid
Copy link
Contributor

@kenza-bouzid kenza-bouzid commented May 27, 2022

To avoid plotting top and bottom tiles by reading them from png files, we now select these tiles on the fly for each batch in min and max heaps and then plot them from their respective tensors. Main contributions:

  • Create TopBottomHandler, SlideNode and TileNode classes
  • Select top and bottom slides on the fly based on probability scores and their respective top and bottom tiles sorted by attention scores
  • Gather results across devices by first gathering a shallow copy of the top and bottom slides and then their respective tiles to avoid copying unuseful tiles across gpu devices

@fepegar
Copy link
Contributor

fepegar commented Jun 1, 2022

I've now reverted the bot's commits. Again, I'm very sorry, @kenza-bouzid!

@kenza-bouzid kenza-bouzid reopened this Jun 6, 2022
@kenza-bouzid
Copy link
Contributor Author

I've now reverted the bot's commits. Again, I'm very sorry, @kenza-bouzid!

No worries, thanks @fepegar for the fix!

Copy link
Collaborator

@ant0nsc ant0nsc left a comment

Choose a reason for hiding this comment

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

Great work - just a few minor comments!

kenza-bouzid and others added 6 commits June 6, 2022 14:03
…_utils.py

Co-authored-by: Anton Schwaighofer <antonsc@microsoft.com>
…_utils.py

Co-authored-by: Anton Schwaighofer <antonsc@microsoft.com>
…_utils.py

Co-authored-by: Anton Schwaighofer <antonsc@microsoft.com>
Copy link
Member

@dccastro dccastro left a comment

Choose a reason for hiding this comment

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

Tremendous accomplishment here, @kenza-bouzid! Now the API looks much cleaner and is easier to understand.

@kenza-bouzid kenza-bouzid merged commit 1d96c9b into main Jun 7, 2022
@kenza-bouzid kenza-bouzid deleted the kenzab/refactor_tiles_plotting branch June 7, 2022 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants