-
Notifications
You must be signed in to change notification settings - Fork 28
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
use ÷ #91
use ÷ #91
Conversation
Codecov Report
@@ Coverage Diff @@
## master #91 +/- ##
=======================================
Coverage 99.75% 99.75%
=======================================
Files 49 49
Lines 2409 2409
=======================================
Hits 2403 2403
Misses 6 6
Continue to review full report at Codecov.
|
@@ -288,7 +288,7 @@ function _down_round( | |||
) where {D} | |||
out = val ./ down | |||
# for non-divisors make dim a multiple of 2 | |||
fun = out -> out == round(out) ? out : 2 * round(out / 2) | |||
fun = out -> out == round(out) ? out : 2 * (out ÷ 2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, in case you're not aware of the differences: floor(Int, dim1/down)
returns an Int
no matter what dim1
or down
is. On the other hand, the return type of dim1 ÷ down
is not necessarily an Int
. (For example, 2.5 ÷ 2 === 1.0
).
This could sometimes cause a performance issue, especially for functions with the existence of an if-branch. As summarised in the performance tips, fun
should be type stable to get rid of the runtime performance overhead.
out = rand(Float64, 64, 64, 30);
f_unstable = out -> out == round(out) ? out : 2 * floor(Int, out/2)
f_stable = out -> out == round(out) ? out : 2 * (out ÷ 2)
@btime f_unstable.($out); # 431.927 μs (14 allocations: 960.58 KiB)
@btime f_stable.($out); # 409.557 μs (5 allocations: 960.16 KiB)
You can call @code_warntype f_unstable(out)
and will see that the return type is inferred as Union{Int, Float64}
, which is not type stable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnnychen94 thanks very much for these tips! i just learned about ÷
and got overly excited about using it, not realized about Float÷Int. i'm about to submit a PR that i hope will address these issues.
@@ -288,7 +288,7 @@ function _down_round( | |||
) where {D} | |||
out = val ./ down | |||
# for non-divisors make dim a multiple of 2 | |||
fun = out -> out == round(out) ? out : 2 * round(out / 2) | |||
fun = out -> out == round(out) ? out : 2 * (out ÷ 2) | |||
out = fun.(out) | |||
dd = dd .* down | |||
return Int.(out), dd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary intermediate memory allocation can also be a root of performance issue:
out = rand(Float64, 64, 64, 30);
function f1(out)
fun = out -> out == round(out) ? out : 2 * (out ÷ 2)
out = fun.(out) # unnecessary memory allocation
return Int.(out) # memory allocation
end
function f2(out)
fun = out -> out == round(out) ? out : 2 * (out ÷ 2)
# the @ macro will fuse all the operations and thus only allocate memory once
return @. Int(fun(out))
end
julia> @btime f1($out);
677.139 μs (4 allocations: 1.88 MiB)
julia> @btime f2($out);
502.425 μs (2 allocations: 960.08 KiB)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for this tip too. in this particular case it is only applied to small tuples, not big arrays, but i will change it anyway to remind me to use @.
more in the future. i really appreciate it.
No description provided.