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

Let user specify Integer ->Float conversion type #628

Closed
wants to merge 19 commits into from

Conversation

jeremiahpslewis
Copy link
Contributor

Based on these related issues: FluxML/Flux.jl#2305, FluxML/Zygote.jl#1445, JuliaGPU/GPUArrays.jl#484

it seems like it might be useful to have a bit more control over whether Int projections are mapped to Float64 or another AbstractFloat subtype. While this might be possible via overloading, it would be nice to know that one hasn't broken any ChanRulesCore.jl functionality by overloading the method. Especially for working with GPUs, greedy type conversion to Float64 seems to be problematic.

This PR introduces a local user preference int2float via which a Float type can be specified. I have also adjusted the tests so that they can be run with different preference settings.

The problem which this PR solves, in a nutshell:

using ChainRulesCore

# Second test fails
ProjectTo(1.0f0)(2) === 2.0f0
ProjectTo(1)(2.0f0) === 2.0f0

# Both tests pass (after restart)
ChainRulesCore.set_int2float!("Float32")
ProjectTo(1.0f0)(2) === 2.0f0
ProjectTo(1)(2.0f0) === 2.0f0

Disclaimer I'm new to this package and imagine that this may be the wrong way to fix the problem, so please don't hesitate to close this PR if I've missed something.

src/preferences.jl Outdated Show resolved Hide resolved
src/preferences.jl Outdated Show resolved Hide resolved

# Set it in our runtime values, as well as saving it to disk
@set_preferences!("int2float" => int2float_type)
@info("New int2float type set; restart your Julia session for this change to take effect!")
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you know already, but one can set preferences before loading a package by specifying the package via its UUID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I've used this in running the tests, anywhere else where this would be helpful?

test/projection.jl Outdated Show resolved Hide resolved
Copy link
Member

@oxinabox oxinabox left a comment

Choose a reason for hiding this comment

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

this scares me. So do most uses of preferences to be fair.
I can't see how this can be safely used.
If one package assumes one behavour and another another then it will error.

Julia provides a function, float, to get the reference floating point type.

Though in most cases it is the chainrules conception to refuse to deal with integers.
because they represent discrete quantities.
And we treat types as having semantic meaning, not merely for computation convieience.
(See also how we treat structurally sparse array types. We don't let their derivatives have nonzeros in the wrong places)
I would have expected

ProjectTo(1)(2.0f0) === NoTangent()

@jeremiahpslewis
Copy link
Contributor Author

@oxinabox Agree, the more I work on this PR, the less I like it. I've opened a new one with only the added tests, so at least the behavior is documented...

@jeremiahpslewis
Copy link
Contributor Author

Superseded by #629

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.

3 participants