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.

Sign up for GitHub

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

Jump to bottom

SliceNdLayer now uses GatherLayer to get the slices. #635

Merged
merged 14 commits into from
Sep 16, 2021
Merged

SliceNdLayer now uses GatherLayer to get the slices. #635

merged 14 commits into from
Sep 16, 2021

Conversation

robin-p-schmitt
Copy link
Contributor

@robin-p-schmitt robin-p-schmitt commented Sep 7, 2021 •
edited
Loading

The start argument of SliceNdLayer can now be of any shape and not only of shape (B,) as before. I am not 100% sure if we need any asserts to check for shapes or so. But I think it should work for any non-exotic shapes.

@Zettelkasten #625

@robin-p-schmitt robin-p-schmitt marked this pull request as draft September 7, 2021 12:33
@Zettelkasten Zettelkasten self-requested a review September 7, 2021 13:33
@robin-p-schmitt

This comment has been minimized.

Sign in to view
albertz
albertz reviewed Sep 7, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 7, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 7, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 7, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 7, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 7, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
@robin-p-schmitt robin-p-schmitt force-pushed the schmitt-slicend-use-gatherlayer branch from d5d46f6 to 4ccf73b Compare September 8, 2021 14:10
@robin-p-schmitt

This comment has been minimized.

Sign in to view
@albertz

This comment has been minimized.

Sign in to view
albertz
albertz reviewed Sep 12, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 12, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 12, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
@robin-p-schmitt robin-p-schmitt force-pushed the schmitt-slicend-use-gatherlayer branch from 4ccf73b to fad90d0 Compare September 14, 2021 07:13
albertz
albertz reviewed Sep 14, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 14, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 14, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 14, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 14, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 14, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 14, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 14, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
@robin-p-schmitt robin-p-schmitt force-pushed the schmitt-slicend-use-gatherlayer branch from a4f220d to 0e563b3 Compare September 16, 2021 09:18
@robin-p-schmitt
Copy link
Contributor Author

This might also be interesting for @mmz33

albertz
albertz reviewed Sep 16, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 16, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 16, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 16, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 16, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
albertz
albertz reviewed Sep 16, 2021
View reviewed changes
returnn/tf/layers/basic.py Outdated Show resolved Hide resolved
@albertz
Copy link
Member

albertz commented Sep 16, 2021

Despite my remaining comments, I think this PR is ready, or not? You should then mark it as "ready for review".

@robin-p-schmitt robin-p-schmitt marked this pull request as ready for review September 16, 2021 12:47
@robin-p-schmitt robin-p-schmitt requested a review from a team as a code owner September 16, 2021 12:47
@albertz albertz merged commit 5eb93b5 into rwth-i6:master Sep 16, 2021
@albertz
Copy link
Member

albertz commented Sep 16, 2021

Looks good now. I merged it now.

In case you see further problems, open a new PR with proposed changes/fixes.

And also for your follow up changes/extensions, please open a new PR.

@robin-p-schmitt robin-p-schmitt deleted the schmitt-slicend-use-gatherlayer branch September 17, 2021 08:16
This was referenced Sep 21, 2021
Merged
Merged
Closed
@robin-p-schmitt robin-p-schmitt mentioned this pull request Nov 24, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albertz albertz albertz approved these changes

@Zettelkasten Zettelkasten Awaiting requested review from Zettelkasten

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

Successfully merging this pull request may close these issues.
3 participants
@robin-p-schmitt @albertz @Zettelkasten