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

Complex-valued gradients broken #117

Closed
mfherbst opened this issue Jan 26, 2020 · 7 comments · Fixed by #119
Closed

Complex-valued gradients broken #117

mfherbst opened this issue Jan 26, 2020 · 7 comments · Fixed by #119

Comments

@mfherbst
Copy link

Due to the changes introduced in #112 for fixing #97, real-valued objective functions of complex parameters (thus complex-valued gradients) are broken.

For example:

x0 = randn(ComplexF64, 3)
OnceDifferentiable(x -> sum(abs2, x), x0)

does not work because of an attempt to copy the complex x0 into a real-valued array allocated in abstract.jl:21:

ERROR: InexactError: Float64(-0.3704615651702863 - 0.8842199776298206im)
Stacktrace:
 [1] Real at ./complex.jl:37 [inlined]
 [2] _broadcast_getindex_evalf at ./broadcast.jl:630 [inlined]
 [3] _broadcast_getindex at ./broadcast.jl:603 [inlined]
 [4] getindex at ./broadcast.jl:563 [inlined]
 [5] macro expansion at ./broadcast.jl:909 [inlined]
 [6] macro expansion at ./simdloop.jl:77 [inlined]
 [7] copyto! at ./broadcast.jl:908 [inlined]
 [8] copyto! at ./broadcast.jl:863 [inlined]
 [9] copy at ./broadcast.jl:839 [inlined]
 [10] materialize(::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1},Nothing,Type{Float64},Tuple{Array{Complex{Float64},1}}}) at ./broadcast.jl:819
 [11] x_of_nans(::Array{Complex{Float64},1}, ::Type) at .julia/packages/NLSolversBase/eVJ9t/src/NLSolversBase.jl:60
 [12] alloc_DF at .julia/packages/NLSolversBase/eVJ9t/src/objective_types/abstract.jl:21 [inlined]
 [13] OnceDifferentiable at .julia/packages/NLSolversBase/eVJ9t/src/objective_types/oncedifferentiable.jl:19 [inlined] (repeats 2 times)
 [14] top-level scope at REPL[21]:1
@antoine-levitt
Copy link
Contributor

Hm, Optim.jl/test/multivariate/complex.jl contains similar codes, that should also fail. @pkofod can you revert?

@pkofod
Copy link
Member

pkofod commented Jan 27, 2020

We should have had tests like that here. Sorry about that.

@mfherbst
Copy link
Author

No worries. Thanks for the quick reply!

@pkofod
Copy link
Member

pkofod commented Jan 27, 2020

If you're looking for a quick fix, you can just specify the input yourself, but if I cannot find a fix, I'll just revert it(see #118) but this should work, right?

OnceDifferentiable(x -> sum(abs2, x), x0, 0.0, x0) #x-like, F-like, G-like

@mfherbst
Copy link
Author

Yes that workaround works, thanks.

@pkofod
Copy link
Member

pkofod commented Jan 27, 2020

Yes that workaround works, thanks.

I didn't intend for that to be required for complex input though, so it's only until I get to fix this. At the very least I can special case it for !isreal input. Again, sorry for the inconvenience.

@mfherbst
Copy link
Author

No worries. It's easy to overlook "special" cases such as that. Best lesson to be learned is to really put such stuff into test cases and make sure it never happens. 😄

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 a pull request may close this issue.

3 participants