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

allow printing using at-printf #511

Merged
merged 2 commits into from
Apr 7, 2021
Merged

Conversation

ranocha
Copy link
Contributor

@ranocha ranocha commented Mar 22, 2021

I implemented specializations of Printf.tofloat to allow printing (the value of) dual numbers. This is useful when differentiating functions that print some information during the computation, cf. https://discourse.julialang.org/t/forwarddiff-jl-and-printf/57694.

I implemented specializations of `Printf.tofloat` to allow printing (the `value` of) dual numbers.
This is useful when differentiating functions that print some information during the computation, cf.
https://discourse.julialang.org/t/forwarddiff-jl-and-printf/57694.
@codecov-io
Copy link

codecov-io commented Mar 22, 2021

Codecov Report

Merging #511 (ca8efc0) into master (86fee75) will decrease coverage by 4.73%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #511      +/-   ##
==========================================
- Coverage   89.36%   84.62%   -4.74%     
==========================================
  Files          10        9       -1     
  Lines         846      826      -20     
==========================================
- Hits          756      699      -57     
- Misses         90      127      +37     
Impacted Files Coverage Δ
src/dual.jl 71.88% <0.00%> (-7.85%) ⬇️
src/prelude.jl 37.50% <0.00%> (-56.25%) ⬇️
src/gradient.jl 98.86% <0.00%> (-0.03%) ⬇️
src/jacobian.jl 99.28% <0.00%> (-0.02%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86fee75...ca8efc0. Read the comment docs.

@KristofferC
Copy link
Collaborator

Is this really correct? Compare with e.g. complex numbers:

julia> Printf.tofloat(1.0 + im)
ERROR: InexactError: Float64(1.0 + 1.0im)

Why shouldn't this give the same error?

@ranocha
Copy link
Contributor Author

ranocha commented Mar 22, 2021

My reason for implementing it like this is something like the following situation. Imagine someone wrote a performant numrical algorithm that can take quite some time in generic Julia code so that AD can just work out of the box. The only problematic part is that some @printf("...%f/%e...") statements are included, which will fail for ForwardDiff.Dual numbers. The people writing this algorithm do not worry about ForwardDiff.jl or dual numbers, since they do not use it inside their package. Hence, it does not seem to be practical to include a dependency on ForwardDiff.jl to just throw ForwardDiff.value into every @printf statement.

In contrast to dual numbers used for AD, complex numbers usually appear in other situations and people will know more often when they need to support them, at least in my field of interest. Is there a better solution to this problem?

@ranocha
Copy link
Contributor Author

ranocha commented Mar 22, 2021

I don't want to appear rude or demanding and I have never contributed to ForwardDiff.jl before, so it's not up to me to make any requests. However, I would like to know how to approach a situation where people have coded a highly complicated algorithm working on Real numbers, using @printf statements, that should be differentiated (not from within the same package). Since ForwardDiff.Dual numbers identify as Real numbers, I would expect them to work with @printf("...%e/%f..., ...) statements.

@sloede
Copy link

sloede commented Mar 25, 2021

Are there any updates on the status of this PR yet?

Since ForwardDiff.Dual numbers identify as Real numbers, I would expect them to work with @printf("...%e/%f..., ...) statements.

I second that statement. The difference to complex numbers is that they do not identify as Real, thus I do understand that they cannot automatically convert to a floating point value by Printf. However, since Printf is modeled after the C function, which does not have support for ForwardDiff.Dual, there is probably no chance that we will have explicit support for duals in Printf anytime soon. Given that ForwardDiff.Dual effectively works as a drop-in replacement for Float64, I think it would be an intuitive decision (and make life easier for users of ForwardDiff.jl) to support printing dual numbers as floats in Printf.

@ranocha
Copy link
Contributor Author

ranocha commented Apr 7, 2021

Bump. Any thoughts on this?

@testset "@printf" begin
for T in (Float16, Float32, Float64, BigFloat)
d1 = Dual(one(T))
@test_nowarn @printf("Testing @printf: %.2e\n", d1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make more sense to use @sprintf and check the result?

Copy link
Contributor Author

@ranocha ranocha Apr 7, 2021

Choose a reason for hiding this comment

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

Thanks, that's a good suggestion! I added an additional test using @sprintf where the resulting string is checked (see below).

@KristofferC KristofferC merged commit 807b0ec into JuliaDiff:master Apr 7, 2021
@KristofferC
Copy link
Collaborator

I personally think this behavior is slightly confusing but people seem to like it so 🤷‍♂️

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.

4 participants