-
Notifications
You must be signed in to change notification settings - Fork 116
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
[FEATURE REQUEST] support for complex dtype #364
Comments
In CircuitPython, we enable complex values on any "full build", but never enable the cmath library. My gut tells me that users of CP don't need complex ulab arrays. When it comes to FFT, spectrum is more useful than the complex fft values themselves. Since you bring up code size increase, it's worth remarking that the current CI of your master branch is not able to check whether additions to ulab potentially over-fill the firmware space of our various boards. |
I'm not the most familiar with circuit pythons inner workings, but would this kind of proposal make more sense as a library/module proposal rather than adding another dtype into Circuit Python? I'm initially seeing it as sort of the same way Numpy has a different set of dtypes than Python does. |
But this is something that I can't possibly check on this side: I have no idea, which (extra) modules you compile into |
That's right, but the I tried to point out that the set of |
I see what you mean by saying it's hard to decouple from the core and especially ulab's functions--with the emphasis that completely real input can generate complex output. But the increase in size by adding a complex dtype is really large. Maybe it's worth it though for these use cases. Looking at the libraries which do 'heavy' math in the python ecosystem--a majority of them use Numpy to recast the data in a complex friendly and efficient math fashion ( I'm wondering if this same approach would be beneficial here. But that suggestion is no small suggestion either. It would have to be written, supported, your suggested issues involving FPUs are still there, and so on. But it could offer a solution and sidestep the memory issue. And again, I could be completely not seeing something core to the CircuitPython version when it comes to this issue. |
But However, Moreover, with an extra library, we cannot avoid the problem of binary operators. In
At the moment, when
where If you care to see the declaration, here it is: Lines 89 to 111 in 6d852a3
micropython-ulab/code/ndarray.c Line 1716 in 6d852a3
What you are suggesting, in effect, is to split the complex arrays into a real and imaginary array by means of some sort of wrapper. This is OK as long as you just want to treat the return values, but I don't know how it would work, when you want to pass arguments. Also, a recurring issue/request here is
That is a nasty can of worms, and I haven't the foggiest idea, how nasty it is.
I am not against such an approach, but we have to iron out the details. |
Ah this is extremely clarifying, thank you. I feel I'm beginning to understand the core of this issue. I wish I could more rapidly come to speed, but that's how it goes. Also thank you for linking me to the source examples, and taking the time to organize it all. And the problem of real numbers generating complex results is an important issue. At the same time I agree full numpy parity isn't possible and should be the motivation.
Can another module override this construct or provide it's own added dtype and safely return it? That way only if the other module is present would that construct have the 'added' complex cases for the binary operator? It would leave (I'm exclusively trying to see the scale of the problem, rather than assume I see the solution.) |
Not at all. Discussions are useful, when the points are clearly articulated. Besides, having a sounding board is really beneficial, especially, in complex cases like this (pun intended).
You want to sort of sub-class the present definition of the The thing is,
What could be done is perhaps the following:
This all is a fair amount of work, but such a solution would, on the one hand, not upset the status quo, and wouldn't take extra space in those cases, when only real numbers are needed, and on the other hand, it would still cut the Gordian knot of the complex |
I have opened a draft under #366. Implementation-specific details should probably be discussed there. |
I have no formal branch here but on my local copy I added the following dtypes: int32, uint32 and int64. |
You should fork the repository, and commit your code. It is hard to comment on what you are doing, if we can't see it. |
I made a fork with committed my changes. |
I would like to discuss the ramifications of adding support for complex arrays in
ulab
.At the beginning, the Fourier transform was the single function that would have required complex arrays, but it was possible to just pass and return two real arrays that stood for the real and imaginary parts. This was not very elegant, but still acceptable. However, there have been at least two feature requests recently that would result in functions that lead out of the domain of real numbers.
Namely,
linalg.eig
for asymmetric matrices (and SVD?) #363The problem is that these functions could potentially return complex arrays, even if the input was real.
Adding a new
dtype
increases the firmware size, because thedtype
s have to be resolved in C; in particular, the biggest contributor is probably the set of binary operators, because there one has to tackle all possible combinations, therefore, this contribution to the firmware size scales with the square of the number ofdtypes
. At the moment, we have 5 and a halfdtypes
((u)int8
,(u)int16
,float/double
, plusbool
, which is actually anuint8
with a special flag), hence we deal with 25 combinations. By the addition ofcomplex
, we would end up with 36 combinations.There are a number of functions that would not have to support
complex
. E.g.,(arg)min
,(arg)max
,(arg)sort
,median
,clip
,fmin
etc. With these considerations, a rough estimate is 30% extra in the firmware size, if we addcomplex
as adtype
.Before jumping off the deep end, here are a couple of questions:
complex
can be excluded from the firmware, then there would be two tiers, where the behaviour of thefft
function depends on the switch. That is highly undesirable.complex
, as, e.g., infft
?complex
is available, then that is the largest set of numbers, therefore, the function pointed to by the function pointer must return a complex number for all types. However, complexes can't just be passed to certain math functions in a haphazard manner. That is, whymicropython
implements its owncmath
module.)complex
only as a return type, i.e., to allow a function to return a complex, but not as an argument, except forreal
,imag
, andfft
?The text was updated successfully, but these errors were encountered: