Skip to content

Set "priority" annotations in SimpleShuffleLayer-based __init__ functions#7846

Merged
mrocklin merged 9 commits intodask:mainfrom
rjzamora:set-split-priority
Jul 1, 2021
Merged

Set "priority" annotations in SimpleShuffleLayer-based __init__ functions#7846
mrocklin merged 9 commits intodask:mainfrom
rjzamora:set-split-priority

Conversation

@rjzamora
Copy link
Member

@rjzamora rjzamora commented Jun 29, 2021

Possible alternative to #7826

@rjzamora
Copy link
Member Author

@mrocklin - Is this what you had in mind? If not, I'll be happy to revise.

@rjzamora rjzamora marked this pull request as draft June 29, 2021 20:24
Copy link
Contributor

@madsbk madsbk left a comment

Choose a reason for hiding this comment

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

Looks good @rjzamora nice work. I agree with @mrocklin, it would be good with a more general test but I don't think you should put too much work into it.

@rjzamora
Copy link
Member Author

it would be good with a more general test but I don't think you should put too much work into it.

I actually started looking into this, and I am not convinced the current version of this PR is "working" - So I do plan to investigate a bit more today.

@mrocklin
Copy link
Member

mrocklin commented Jun 30, 2021 via email

@rjzamora rjzamora marked this pull request as ready for review June 30, 2021 14:51
@mrocklin
Copy link
Member

mrocklin commented Jul 1, 2021

@madsbk @rjzamora which is better this or #7826 ? I'm happy with either. We should get one in.

@rjzamora
Copy link
Member Author

rjzamora commented Jul 1, 2021

which is better this or #7826 ? I'm happy with either. We should get one in.

I have no real preference - If you prefer #7826, I suggest adding the test from this PR

@madsbk
Copy link
Contributor

madsbk commented Jul 1, 2021 via email

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.

4 participants