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

Pixel Shuffle layer #2563

Merged
merged 13 commits into from Feb 26, 2021
Merged

Pixel Shuffle layer #2563

merged 13 commits into from Feb 26, 2021

Conversation

iamshnoo
Copy link
Member

@iamshnoo iamshnoo commented Aug 11, 2020

Pixel Shuffle layer

To compare PyTorch/numpy implementation with mlpack/armadillo implementation of the functionality, check this Google Colab notebook.

pixel_shuffle

@sreenikSS
Copy link
Contributor

Looks good to me. Just take care of the conflicts and I'll merge it.

Copy link

@mlpack-bot mlpack-bot bot left a comment

Choose a reason for hiding this comment

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

Second approval provided automatically after 24 hours. 👍

@sreenikSS
Copy link
Contributor

Could you fix this once more?

Copy link
Member

@birm birm left a comment

Choose a reason for hiding this comment

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

Marking changes requested until build is fixed :)

@kartikdutt18
Copy link
Member

Resolving the conflict here.

@iamshnoo
Copy link
Member Author

Any clue regarding the failing tests on this one?
When it was approved previously, the build was passing with all tests. However, currently it doesn't compile !? 👀
Not sure if there is something I need to add from my end.

@zoq
Copy link
Member

zoq commented Oct 12, 2020

Any clue regarding the failing tests on this one?
When it was approved previously, the build was passing with all tests. However, currently it doesn't compile !? 👀
Not sure if there is something I need to add from my end.

Can you merge in or rebase against the latest master branch?

@rcurtin
Copy link
Member

rcurtin commented Oct 16, 2020

Looks like this one is pretty close to ready---should I put it in the mlpack 3.4.2 milestone? I'm hoping to do a quick release in the next couple days. :)

@iamshnoo
Copy link
Member Author

Yep, from my end, the implementation of this feature is complete. All it would need is pulling in the recent changes, before we can merge this. So, you can add it to the 3.4.2 milestone, yes :)

@zoq zoq added this to Need Review in PR Tracking Jan 12, 2021
@zoq
Copy link
Member

zoq commented Jan 12, 2021

Merged the latest master banch in, let's see if the CI pipeline comes back green.

@abhinav-anand-addepar
Copy link
Member

@zoq Can i take over this PR?

@zoq
Copy link
Member

zoq commented Feb 4, 2021

@zoq Can i take over this PR?

Let's see what @iamshnoo says, with the recent switch from boost serialization to cereal, we have to update the PR.

@iamshnoo
Copy link
Member Author

iamshnoo commented Feb 5, 2021

It's fine with me if @abh2k would like to update the PR. I guess the only change would be in the serialize() method, right?

@abhinav-anand-addepar
Copy link
Member

Thanks. I will update the pr.

@birm birm merged commit 06ed1d6 into mlpack:master Feb 26, 2021
@birm
Copy link
Member

birm commented Feb 26, 2021

Oh cool, since the followup, #2833 was merged, github automatically tagged this as merged too!

This was referenced Oct 14, 2022
@rcurtin rcurtin mentioned this pull request Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
PR Tracking
Need Review
Development

Successfully merging this pull request may close these issues.

None yet

7 participants