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

FastMath operations #174

Closed
MikeInnes opened this issue Apr 20, 2020 · 4 comments · Fixed by #185
Closed

FastMath operations #174

MikeInnes opened this issue Apr 20, 2020 · 4 comments · Fixed by #185
Labels
help wanted Extra attention is needed missing rule

Comments

@MikeInnes
Copy link

It would be good to have support for @fastmath, especially the functions that ccall libm.

Not sure if it's the right approach here, but in Zygote we did this by looping over the usual DiffRules.

This is a bit of a blocker for FluxML/Zygote.jl#366, because we want to remove DiffRules as part of that, but currently doing so would cause a regression on this.

@nickrobinson251
Copy link
Contributor

nickrobinson251 commented Apr 20, 2020

if we wanted to do the simplest swap for DiffRules, then probably requires JuliaDiff/ChainRulesCore.jl#127

@oxinabox
Copy link
Member

oxinabox commented Apr 20, 2020

I would not do it that way, and we don't really need to.
Since we don't need a dynamic list of of rrules,
because the rules we need to know if they exist or not are define in this package.

We can just metaprogram them out directly inside ChainRules.
looping over Base.FastMath.fast_op
or just by writing the list we have rules for out and looping over-that.

@nickrobinson251 nickrobinson251 added help wanted Extra attention is needed missing rule labels Apr 29, 2020
@oxinabox
Copy link
Member

oxinabox commented Apr 30, 2020

For reference the following almost works.
It makes 1 mistake -- it overwrites the rules for mod

Create a new file called .e.g fastmath.jl next to src/rulesets/Base/base.jl
with the contents:

using ChainRulesCore
src_code = String(read(joinpath(@__DIR__, "base.jl")))
src_code = "begin $src_code end"
src_ast = Meta.parse(src_code)
block_asts = [x for x in src_ast.args if x isa Expr]  # don't want anything that don't have subexpressions
transformed_asts_once = Base.FastMath.make_fastmath.(block_asts)
new_asts = transformed_asts_once[block_asts .!= transformed_asts_once]
expanded_asts = macroexpand.(Ref(@__MODULE__), new_asts)
transformed_asts = Base.FastMath.make_fastmath.(expanded_asts)
eval.(transformed_asts)

because make_fastmath walks ASTs and then replaces things with there fast versions including in the typeof line.

I don't think this is entirely an unreasonable way to do this.
It is the thing we want.
Make rule(::typeof(fast_foo) have the same body as the rule for foo but with all operations replaced with fast_ operations

@oxinabox
Copy link
Member

oxinabox commented Apr 30, 2020

Perhaps as less horrifying way is to make a quote block inside base.jl
which contains only things we want to do make_fastmath on.

fastable_ast = quote
    @scalar_rule ....
    ...
    ...
    rule(::typeof...)
    ...
end

eval(Base.FastMath.makefast(fastable_ast))  # declare fastmath versions
eval(fastable_ast)  # also the original nonfast versions (do this second incase mistakenly tranformed an orignal)

That seems less scary,
and we can manually exclude mod by not putting it inside the quote.
Also it doens't break Revise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed missing rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants