Vote: should we add Complex number support as a feature of ForwardDiff.

[GiggleLiu]

• upvote: 👍 , for supporting it in ForwardDiff.jl
• downvote: 👎 , for supporting it in another package called GenericComplex.jl

Related PR:

• complex number support #455

Related Issues:

• Support for complex numbers #157
• StackOverflowError with complex sqrt #324
• support for complex valued function #364

[GiggleLiu]

A summary of discussion in slack:

Yingbo Ma:sheepy:  12:11 AM
I don't think it really fits in ForwardDiff.jl though

JinGuo Liu  12:36 AM
well, wondering why? I think supporting complex numbers are important. (as issues suggests) (edited) 

Yingbo Ma:sheepy:  12:38 AM
Because those functions have nothing to do with ForwardDiff or Dual . They are just generic fallbacks
12:38
I suggest you to make another package for that.

JinGuo Liu  12:43 AM
Hmm, it has practical significance. It is only a tiny patch that does not cause any overhead.
12:43
and from the user perspective, it belongs to a part of ForwardDiff. Like complex numbers are in Julia Base.
12:45
using ForwardDiff, ComplexForwardDiff

Yingbo Ma:sheepy:  12:46 AM
It should be more like using ForwardDiff, GenericComplex

JinGuo Liu  12:46 AM
Hmm, BTW, it has to be in ForwardDiff
12:46
because, it pirates
12:46
otherwise

Yingbo Ma:sheepy:  12:46 AM
Just like how you use using ForwardDiff, GenericLinearAlgebra
12:47
Those functions don't need to depend on Dual at all, just type it as ::Complex{<:Real} (edited) 

JinGuo Liu  12:47 AM
still pirates?

Yingbo Ma:sheepy:  12:48 AM
I don't get why that pirates.

JinGuo Liu  12:49 AM
It changes the behavior of Base function on Base types.
12:49
e.g. log(x::Number)  will cause other packages break.

Yingbo Ma:sheepy:  12:49 AM
Extending base functions on abstract types isn't bad, assuming your implementation is valid.

JinGuo Liu  12:49 AM
If it is defined in ForwardDiff on Dual only, it is fine.
12:50
What if some other package also defines log  on generic types?
12:50
Like the adjoint in autodiff packages. now it is believed not a good design pattern.
12:50
foo'(args...)
12:51
Base function adjoint  defined on function  types.
12:51
BTW: NiLang removed this pirate behavior in a recent release. (edited) 

Yingbo Ma:sheepy:  12:51 AM
Then they can contribute to GenericComplex
12:52
https://github.com/JuliaLinearAlgebra/GenericSVD.jl#genericsvdjl
It internally overloads several Base functions such that existing methods (svd, svdfact and svdvals) should work directly.
12:53
Defining foo' is bad because that abuses adjoint .

JinGuo Liu  12:53 AM
We avoid doing it whenever there are alternatives. Generic SVD is special. (edited) 

Yingbo Ma:sheepy:  12:54 AM
Why is that special?

JinGuo Liu  12:54 AM
Well, u r right,  I don't think they should pirate. (edited) 

Yingbo Ma:sheepy:  12:56 AM
I mean if you don't want to use that particular method, you just don't install that package.

JinGuo Liu  12:56 AM
in https://github.com/GiggleLiu/BackwardsLinalg.jl , I removed the import of svd from Base, someone complaints.
12:57
I think another criteria is "negligible overhead" in using time and first use time.
12:58
not "related to Dual". I can't agree with it.
12:58
Let's open an issue for voting!

Yingbo Ma:sheepy:  12:58 AM
How is that related to Dual expect the function signature.

JinGuo Liu  1:00 AM
I mean, it is not a criteria.
1:00
for not importing some important new feature.

Yingbo Ma:sheepy:  1:00 AM
It can just be a generic implementation that just so happens work with Dual.

[keefehuang]

Hi, I am currently working on a project that requires autodiff with complex support (even for basic functions would be nice). I would greatly appreciate if this could be worked on in this repo. But, if this doesn't happen, I would also like to help out in the new repo.

[ChrisRackauckas]

Of course everyone wants ForwardDiff to support complex numbers, but a vote doesn't fix the fundamental issues with the approach. The right thing to do is to fix the approach. I don't think everyone who uses ForwardDiff wants all other packages to have to recompile just because a few people needed complex support, and that's exactly what this would do. Anything that hit these dispatches on log, exp, etc. would now recompile. This could lead to some... massive compile times. It also means that some code can work differently if ForwardDiff exists or doesn't. And these changes now have to be done lock-step with Base: if Base.log changes, we probably want to change this. All of this suggests that this can be a fine approach, but the code shouldn't live here, it needs to live in Base as either extensions to the existing dispatches or alternative dispatches.

[ChrisRackauckas]

Or just make an AbstractComplex package for now with these dispatches and use it where you need it and call it a day. But putting something with these crazy side effects and maintenance issues into ForwardDiff to help on an edge case antithetical to the fact that this library is quite stable otherwise.

[GiggleLiu]

Yeah, this is in fact a vote about
A: add the complex number support into ForwardDiff directly, however, need to copy some codes from Base.complex.
B: setting up a new package, pirate ForwardDiff, however, ForwardDiff itself is more elegant (Also, the maintainace pressure is shifted from ForwardDiff to the extension package owner).

@ChrisRackauckas

all other packages to have to recompile just because a few people needed complex support.
Anything that hit these dispatches on log, exp, etc. would now recompile. This could lead to some... massive compile times.

I haven't heard about this. I have rewriten complex valued functions in NiLang too, but haven't seen any problem of doing this.
Is there an issue or code snipt about where does the compiling happens?

if Base.log changes, we probably want to change this.

I believe Base.complex is relatively stable though.

@huangq1234553
Feel free to do anything with the PR #455 , including setting up a ForwardDiff extension package. It will be great if you are willing to maintain it.

[GiggleLiu]

I believe the essential debate is about whether the package maintainer think complex number should be just available.
People in physics field handle complex numbers more often than real numbers, however, most machine learning people never use it. For the same reason, pytorch does not have complex number support at all. This is only a matter of taste.

This vote just reflects "how many people wants this feature eagerly,". In any case, I will support the maintainer's decision. Pytorch is closed ecosystem, but Julia is not, you have a lot ways to add a complex number patch :P.

[ChrisRackauckas]

I believe the essential debate is about whether the package maintainer think complex number should be just available.

That's just a strawman and has nothing to do with what's being discussed. I want complex number support in ForwardDiff more than you do, therefore I want it done correctly and even led to the first issue about it #157 ? See this is going nowhere: the discussion is not whether to support it, it's how to support it.

Looking at JuliaLang/julia#36030, complex ForwardDiff should already be supported on v1.6. So it's really just a question of backwards compatibility at this point.

[GiggleLiu]

@ChrisRackauckas
Good to know JuliaLang/julia#36030 is merged. Thanks for your guys' effort!

[ChrisRackauckas]

Handled by JuliaLang/julia#36030