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

Make _Py_c_sum(), _Py_c_diff(), etc (elementary operations on Py_complex) - part of the public API #128813

Open
skirpichev opened this issue Jan 14, 2025 · 13 comments
Assignees
Labels
topic-C-API type-feature A feature request or enhancement

Comments

@skirpichev
Copy link
Member

skirpichev commented Jan 14, 2025

Feature or enhancement

Proposal:

Suggested by @vstinner in #124829 (comment).

I would agree, as these routines could be useful (and it seems, they are used by few projects) to implement mathematical functions just like in the cmath module. No alternatives exist.

It's also suggested to use a different convention for arguments: currently we pass them by value. We could use pointers to Py_complex struct instead. (Though my quick tests shows no measurable difference.)

Also, we should decide on naming. For _Py_c_sum() - PyComplex_Add() was suggested. But this looks misleading, as PyComplex_ is a prefix for functions, operating with PyObject* arguments. Perhaps, rather we could instead use Py_complex_add() and Py_complex_add_real() (in the GNU GSL style). Current semi-private functions should be deprecated.

Previous discussions:

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs

@encukou
Copy link
Member

encukou commented Jan 14, 2025

There is an alternative: the C11 double complex type.
AFAICS, the remaining reasons for Python to expose the PyComplex struct are:

  • backwards compatibility
  • interop with other languages (i.e. FFI), including older C. The API should be usable with only a limited set of basic C types (double but not _Complex). But, we don't need to provide arithmetic for this use case.

If users want to use Python API for arithmetic, they should use PyNumber_*.

IMO, we should instead add PyComplex_FromDoubleComplex & PyComplex_AsDoubleComplex (but the From/As functions we already have are more important).

It's a question for the C API WG though, if not for wider discussion.

@skirpichev
Copy link
Member Author

There is an alternative: the C11 double complex type.

Not all compilers support that type. For msvc we have _Dcomplex, that might be an alternative.

IMO, we should instead add PyComplex_FromDoubleComplex & PyComplex_AsDoubleComplex (but the From/As functions we already have are more important).

Maybe rather just one PyComplex_AsDoubles.

@skirpichev
Copy link
Member Author

On another hand, it seems that all PEP 11 platforms have complex numbers in some form. And Py_complex can be coerced both to _Dcomplex and double _Complex.

So, the plan is:

  1. Document that Py_complex can be coerced to native complex types. Thus, PyComplex_AsCComplex() can be used to extract complex type.
  2. Remove recently added mixed-mode arithmetic functions from the public API.
  3. Deprecate old arithmetic functions.

How this sounds, should this be discussed in the C API WG? CC @serhiy-storchaka

We also have API functions to extract real or imaginary components. Maybe we can also hide details of the Py_complex structure? E.g. instead we can say about Py_complex something like C standard says: "has the same representation and alignment requirements as an array type containing exactly two doubles". Then with a wider adoption of the Annex G we can switch to native complex type.

@encukou
Copy link
Member

encukou commented Jan 14, 2025

AFAIK, MSVC's _Dcomplex is the C11 double complex type. (You do need to include <complex.h> for the definition, though, which we might not want to do from <Python.h>.)

Maybe we can also hide details of the Py_complex structure? E.g. instead we can say about Py_complex something like C standard says: "has the same representation and alignment requirements as an array type containing exactly two doubles".

No, Py_complex is a good sructure for inerop. IMO, we should keep it (for external API -- import/export).
Defining the struct in English, rather than in C, doesn't really bring any benefits :)

Then with a wider adoption of the Annex G we can switch to native complex type.

We should add the C complex type. The C API is not just for C and C++ :)

How this sounds, should this be discussed in the C API WG? CC @serhiy-storchaka

Sounds good to me; do ask the WG though.

@skirpichev
Copy link
Member Author

MSVC's _Dcomplex is the C11 double complex type.

No, but it's memory layout seems compatible with double complex (and Py_complex). However, you can't use usual arithmetic ops with _Dcomplex. Thus it's something completely different from Annex G complex type:)

No, Py_complex is a good sructure for inerop. IMO, we should keep it (for external API -- import/export).

I'm not saying it's bad. But keeping fields of this structure public - blocks us from using Annex G double complex instead of Py_complex. I'm not sure it's wise to close this door.

Defining the struct in English, rather than in C, doesn't really bring any benefits :)

double _Complex type has no real/imag fields. If we don't specify that Py_complex type has these fields - we can use native complex type instead. Can we count this as a benefit? (And it will be possible to extract real/imag components without using CPython C-API.)

@serhiy-storchaka
Copy link
Member

We should not use the same prefix PyComplex_ for PyObject * based API and Py_complex based API. This will lead to confusion and may even create name conflicts.

As for passing the Py_complex arguments by pointer instead of by value, it conflicts with the existing API. PyComplex_FromCComplex() takes the Py_complex argument by values and PyComplex_AsCComplex() returns a Py_complex value. Modern compilers should be pretty efficient in passing small structures by value, so I do not expect significant benefit if any.

The structure of Py_complex is documented, so we cannot make it an alias of other complex type which is not compatible with

      typedef struct {
          double real;
          double imag;
      } Py_complex;

We should also take into account compatibility with C++.

@encukou
Copy link
Member

encukou commented Jan 15, 2025

it's memory layout seems compatible with double complex (and Py_complex).

If the layout is compatible, I'd expect that CMPLX(pycomplex.real, pycomplex.imag) and (Py_complex){creal(c11complex), cimag(c11complex)} could have no runtime cost.
Is that not the case?

I'm not saying it's bad. But keeping fields of this structure public - blocks us from using Annex G double complex instead of Py_complex. I'm not sure it's wise to close this door.

Hm, could we switch away from using Py_complex internally, and keep it for the From/As API only?
That is, deprecate public use PyComplexObject, and after a few (or many) releases, make the struct internal and switch from cval to a double complex member.

@skirpichev
Copy link
Member Author

We should not use the same prefix PyComplex_ for PyObject * based API and Py_complex based API.

Sure. Naming like Py_complex_add() and Py_complex_add_real() sounds ok for you?

But I like @encukou idea to drop arithmetic functions from the public API. People could use native complex types on their platform instead. One minor note, custom functions do make sense if our support for complex arithmetic will go beyond the C standard (or, rather, beyond it's implementations). As in my proposal, for example: https://discuss.python.org/t/77073.

for passing the Py_complex arguments by pointer instead of by value, it conflicts with the existing AP [...] I do not expect significant benefit if any.

As I said, my preliminary benchmarks show no measurable effect.

If the layout is compatible, I'd expect that CMPLX(pycomplex.real, pycomplex.imag) and (Py_complex){creal(c11complex), cimag(c11complex)} could have no runtime cost.

No, I meant you can do something like this (on Windows you can use _Dcomplex, as it looks from docs):

Py_complex z = {.real=1.25, .imag=-0.5}; // say this come from PyComplex_AsCComplex()
double complex *zn = (double complex *)(&z);  // in memory layout is same
printf("(%lf%+lf)\n", creal(*zn), cimag(*zn));  // do math with native complex type

In principle, this seems to be working in both directions (i.e. you can pass double complex to PyComplex_FromCComplex()) on most platforms. But that's not backed by the C standard.

Hm, could we switch away from using Py_complex internally, and keep it for the From/As API only? That is, deprecate public use PyComplexObject

BTW, PyComplexObject structure fields aren't documented, just as for PyFloatObject.

@encukou
Copy link
Member

encukou commented Jan 15, 2025

No, definitely don't cast between Py_complex and double complex. Even if it happens to work, a future compiler update can break that. And if it's faster than a proper conversion, that's a missing optimization opportunity for the compiler.

BTW, PyComplexObject structure fields aren't documented, just as for PyFloatObject.

That doesn't really matter. Per PEP-387, “if something is not documented at all, it is not automatically considered private”.

@skirpichev
Copy link
Member Author

Even if it happens to work, a future compiler update can break that.

No, it's something we can rely on, thanks to the C standard; it says: "Each complex type has the same representation and alignment requirements as an array type containing exactly two elements of the corresponding real type; the first element is equal to the real part, and the second element to the imaginary part, of the complex number." (c) C17 §6.2.5p13.

@encukou
Copy link
Member

encukou commented Jan 15, 2025

Structs are not arrays; they may have padding between the members (§6.7.2.1.15). Unlikely in production, but a future sanitizer might well catch it.

One minor note, custom functions do make sense if our support for complex arithmetic will go beyond the C standard (or, rather, beyond it's implementations).

I don't think CPython should not provide and maintain public C API for that. This is not the place to fill holes in libc.

@skirpichev
Copy link
Member Author

Structs are not arrays; they may have padding between the members (§6.7.2.1.15).

Thanks, I miss this. Looks not too likely for simple structure with two doubles, but...

I don't think CPython should not provide and maintain public C API for that. This is not the place to fill holes in libc.

Maintenance cost shouldn't be too high. Without this people lose opportunity to add new mathematical functions with C-API.

Of course, it's a hypothetical scenario so far. In present shape our low-level function should be equivalent (modulo errors somewhere) to most existing Annex G implementations.

I'll open a C-API WG issue, as you suggested.

@skirpichev
Copy link
Member Author

The 3.14b1 is coming, so I'm going to remove newly added semi-private functions for doing mixed-mode arithmetic. If we decide to make public the current low-level API - this can be re-added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants