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

Layer normalisation for images #592

Merged
merged 2 commits into from
Feb 5, 2019
Merged

Layer normalisation for images #592

merged 2 commits into from
Feb 5, 2019

Conversation

MJ10
Copy link
Contributor

@MJ10 MJ10 commented Feb 4, 2019

Resolves #406

As suggested by @MikeInnes and @skariel, added dims argument to function, with default value as 1 and changed type of x to AbstractArray

@MikeInnes
Copy link
Member

LGTM. I think the docstring is a bit confusing. Maybe something like, "Normalises x to mean 0 and standard deviation 1, across the dimensions given by dims. Defaults to normalising over columns."

@MJ10
Copy link
Contributor Author

MJ10 commented Feb 4, 2019

That sounds better, I'll make the changes.

@DhairyaLGandhi
Copy link
Member

Oops, looks like GitHub's Web UI isn't happy about the conflict resolution. Can you take a look at the spurious changes please?

Many thanks!

@MJ10 MJ10 force-pushed the master branch 3 times, most recently from eceaf3e to 8cad99d Compare February 5, 2019 10:20
@MJ10
Copy link
Contributor Author

MJ10 commented Feb 5, 2019 •
edited
Loading

Sorry about the multiple pushes, made a wrong commit by mistake.
I've resolved the conflicts now.

@DhairyaLGandhi
Copy link
Member

Hmm, I see some conflict artefacts in the last push...

Sorry about that

@MJ10
Copy link
Contributor Author

MJ10 commented Feb 5, 2019

Really sorry about that, fixed it.

@DhairyaLGandhi
Copy link
Member

DhairyaLGandhi commented Feb 5, 2019 •
edited
Loading

Absolutely no worries and sorry for the confusion. Thanks for the PR!

Could you add the keyword corrected in std as is there in master currently?
Looks good to me!

@MJ10
Copy link
Contributor Author

MJ10 commented Feb 5, 2019

I missed that while resolving the conflicts, sorry!

@DhairyaLGandhi DhairyaLGandhi merged commit 53875a8 into FluxML:master Feb 5, 2019
@DhairyaLGandhi
Copy link
Member

Thanks a lot!

@chengchingwen chengchingwen mentioned this pull request Feb 9, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
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
@MJ10 @MikeInnes @DhairyaLGandhi