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

Add rules for FastMath #185

Merged
merged 5 commits into from
May 7, 2020
Merged

Add rules for FastMath #185

merged 5 commits into from
May 7, 2020

Conversation

oxinabox
Copy link
Member

@oxinabox oxinabox commented May 6, 2020

Closes #174
(cc @MikeInnes)

Needs tests still.
The code has tests, in that it is still the same basic source as before, so the base cases are tested.
there are no tests of the fast math versions though.
Those tests can be generated in the same way however, if that is acceptable?


This PR also adds some missing tests for things that have fast math varients:

  • expm1
  • binary + and -, /, \, max, min

They are added in the trivial way, by adding them to the loops identical to others of their class.

@oxinabox
Copy link
Member Author

oxinabox commented May 6, 2020

the split of the files was created via

using MacroTools
using ChainRulesCore
src_code = String(read(joinpath(@__DIR__, "old_base.jl")))
src_code = "begin $src_code end"
src_ast = Meta.parse(src_code)
block_asts = [MacroTools.striplines(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)
is_fastable = block_asts .!= transformed_asts_once
transformed_asts_once[is_fastable] .|> println

open(joinpath(@__DIR__, "fastable.jl"), "w") do fh
    println.(Ref(fh), block_asts[is_fastable])
end

# check against:
transformed_asts_once[is_fastable]


open(joinpath(@__DIR__, "base.jl"), "w") do fh
    println.(Ref(fh), block_asts[.!is_fastable])
end

transformed_asts_once[.!is_fastable]

Then a bunch of manual stuff.
The compiler helpfully tells you when you define things twice, so that makes it easier to find where there were rules generated that were for things that did not have fastmath varients

test/rulesets/Base/fastmath_able.jl Outdated Show resolved Hide resolved
test/rulesets/Base/base.jl Outdated Show resolved Hide resolved
test/rulesets/Base/fastmath_able.jl Outdated Show resolved Hide resolved
test/rulesets/Base/fastmath_able.jl Outdated Show resolved Hide resolved
end

@testset "Zero over the point discontinuity" begin
# Can't do finite differencing because we are lying
Copy link
Member

Choose a reason for hiding this comment

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

Curious, what do you mean by this?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is some math involve but basically the defintion of derivative is taking limit of small steps around a point.
It is consider to be not defined if when you take those steps to the left and to the right you get a different answer.
In this case you get lim x->0 +1/x and lim x-> 0 -1/x
which we can say is something like positive and negativ infinity.

There is some extension of the math for deritivatives called the subgradient method.
and one of the outcomes of it is that it is good to treat derivatives that are not defined,
but that are such that from one side the limit is positive and from the other it is negative as being zero.
This is because one of the main uses of the dervative is to find local minima,
which typically are defined as being derivative zero at that point.
But for in cases where the dervative at that point is not define then it is also true that if the derivative on either side is opposite in sign it is also a local minima

Co-authored-by: Matt.jl <matt.brzezinski@invenia.ca>
@oxinabox
Copy link
Member Author

oxinabox commented May 7, 2020

@mattBrzezinski is this good to go?

@oxinabox oxinabox merged commit 2d8b265 into master May 7, 2020
@oxinabox oxinabox deleted the ox/fastmath branch May 7, 2020 22:43
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.

FastMath operations
2 participants