adaptive icon indicating copy to clipboard operation
adaptive copied to clipboard

add support for complex numbers in Learner1D

Open stuartthomas25 opened this issue 4 years ago • 12 comments

Description

I ran into an issue interpolating scattering matrices using adaptive where Learner1D.tell casts complex data to real. The reasoning for this is not immediately apparent so I made a small change that seems to work fine in this case. This is by no means a complete implementation but it seems to function fine with the loss function.

Type of change

Check relevant option(s).

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] This change requires a documentation update

stuartthomas25 avatar Jan 05 '20 05:01 stuartthomas25

Well spotted!

Hm, for sure the original code also looks like a bug (just silently discarding the complex part is never the right thing to do).

akhmerov avatar Jan 05 '20 10:01 akhmerov

Good catch! We should add a test that catches this problem too.

basnijholt avatar Jan 05 '20 11:01 basnijholt

I just remembered that in the past we have always recommended implementing functions that return complex numbers as a vector and add the logic in the loss function. Still, we shouldn't discard the user's input.

basnijholt avatar Jan 06 '20 09:01 basnijholt

I have added a test, @akhmerov or @jbweston, feel free to merge.

basnijholt avatar Jan 06 '20 14:01 basnijholt

How do we treat the rescaling for complex data? I expect that we apply np.ptp somewhere, and that might not work correctly.

akhmerov avatar Jan 06 '20 14:01 akhmerov

Damn, I hadn't thought about that.

I am tempted to just raise an error when using complex numbers, and instead recommend people to use vectors.

basnijholt avatar Jan 06 '20 14:01 basnijholt

@basnijholt status update / are we blocked?

jbweston avatar Apr 28 '20 20:04 jbweston

We could implement some rescaling policy that we consider to be reasonably well suited to complex numbers. For example we could choose the overall scale to be scalar, and rescale the values so that both the real and imaginary parts lie within [0, 1] interval.

akhmerov avatar Apr 28 '20 20:04 akhmerov

I think we should close this because this is better than done using a vector. We should document how to do this though. Probably the FAQ would be a good place to do this.

Otherwise, properly implementing this is probably much more complicated than what this PR does.

basnijholt avatar May 12 '20 22:05 basnijholt

@basnijholt perhaps, but the current rescaling logic doesn't suit the case of complex numbers. For example, if we are computing something that's real down to machine precision, the learner will amplify the noise. Choosing the same scale for real and imaginary parts is more appropriate.

akhmerov avatar May 12 '20 22:05 akhmerov

But someone can just do that using the right loss function.

basnijholt avatar May 12 '20 22:05 basnijholt

I just remembered that in the past we have always recommended implementing functions that return complex numbers as a vector and add the logic in the loss function.

I think we should close this because this is better than done using a vector. We should document how to do this though. Probably the FAQ would be a good place to do this.

Hello Bas, I looked over the docs but I haven't found anything about dealing with complex numbers. Would you be able to provide an example?

Davide-sd avatar Dec 10 '21 10:12 Davide-sd