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

Zero is a real number (Flux.Nil) #1830

Merged
merged 2 commits into from
Feb 5, 2022
Merged

Zero is a real number (Flux.Nil) #1830

merged 2 commits into from
Feb 5, 2022

Conversation

mcabbott
Copy link
Member

To see what CI thinks, this gives the special nil used for outputsize supertype Real.

The reason to do so is that really all the activation functions should be defined for ::Real, not ::Any in the name of sanity, and not ::Number to exclude complex numbers, since they often test x>0 etc.

Maybe this has consequences I don't see yet. They won't work in a function which with signature ::Complex, but nor did they before -- and most complex-valued things accept real numbers too.

@mcabbott mcabbott closed this Jan 13, 2022
@mcabbott mcabbott reopened this Jan 13, 2022
@mcabbott mcabbott mentioned this pull request Jan 13, 2022
@darsnack
Copy link
Member

Right now, if something accepted only Complex and not Real, then Nil would fail. But it seems like we would want to pass it in, and instead define the things that need to be defined for Complex numbers (e.g. conj etc.).

Nil is more like a Missing that subtypes Number. "Zero" is a complex number too 😉.

@mcabbott
Copy link
Member Author

mcabbott commented Feb 5, 2022

I'm not sure I follow your thinking, but let me explain mine, with apologies if this is too basic.

For functions, normally want the widest signature for which they will give correct results. If you call C then perhaps that might be f(::Float64), if you're pretty generic but assume that multiplication is commutative, then ideally you want f(::Union{Real, Complex}), JuliaDiff/ChainRules.jl#540. If you only accept positive numbers, or if you divide up the real line into different implementations, then you do exclude complex numbers (and quaternions) and f(::Real) seems like a sensible way to communicate this.

Of course Flux & friends have many f(::Any) which actually only accept real numbers, but (IMO) this is part of why error messages are so confusing, as something then goes wrong 10 levels deeper.

For types, I think instead you want the narrowest signature which doesn't cause problems? Clearly 0 == false but you can't subtype Bool. You can subtype Integer but it's plausible that'll run into problems -- some functions dispatch on ::Integer to do something categorical, and we want this not to take such paths.. If we make nil isa Real then it will still be accepted by all f(::Number), but also by f(::Union{Real, Complex}).

@darsnack
Copy link
Member

darsnack commented Feb 5, 2022

Looking at my comment, I don't know what I meant either. I think my logic was backwards in a bit of a drive-by comment. I think this is a good PR to merge.

@mcabbott mcabbott merged commit 6b335a4 into FluxML:master Feb 5, 2022
@mcabbott mcabbott deleted the real branch February 5, 2022 03:52
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.

2 participants