Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

GH-29021: Convert umath_linalg to multi-phase init (PEP 489)#29030

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
AA-Turner wants to merge1 commit intonumpy:main
base:main
Choose a base branch
Loading
fromAA-Turner:multi-phase/umath_linalg

Conversation

AA-Turner
Copy link

@AA-TurnerAA-Turner commentedMay 21, 2025
edited
Loading

@AA-TurnerAA-Turnerforce-pushed themulti-phase/umath_linalg branch fromeae5c53 to6876e24CompareMay 22, 2025 05:45
Copy link
Member

@mattipmattip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Please work on a single PR to reduce maintainer review burden. Once we are settled that we wish to make this change, we can revisit the other ones. Does this impact performance, either import time or runtime?

PyModuleDef_HEAD_INIT, /* m_base */
"_umath_linalg", /* m_name */
NULL, /* m_doc */
0, /* m_size */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

For now, in a future PR this can be changed.

Suggested change
0,/* m_size*/
-1,/* m_size*/

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

See also my note in#29025 (comment). Briefly, I was mistaken in my understanding of the documentation,-1 is only valid for single-phase initialisation, and is forbidden for multi-phase.

The safest thing reinitialisation-wise would be to use themultiple interpreter opt-out suggested by CPython. This would enforce that each extension module is only imported & initialised once per process.

However, this causestest_reloading.py::test_full_reimport() to fail. Is that an acceptable trade-off? Multi-phase init (and at some point, isolated modules) opens the door to proper support for reloading/reimportingnumpy.

cc@encukou re the best way to do a gradual migration to multi-phase

In the case ofumath_linalg, we might be fine globals-wise, butmultiarray will definitely need a solution here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'vejust updated the HOWTO, seehere before it gets to main docs.

Basically, to allow re-import, clear the "loaded" flag inPyModuleDef'sm_clear. Note that thiswill do an actual reload of the module (sans C globals), that is,exec will run. (With single-phase, the module dict contents are stashed away and reused. You could emulatethat, but )

The PyMutex stuff in the updated HOWTO is 3.13+ non-limited API, but isn't necessary in the medium term (on CPython you're protected by the GIL; even free-threading builds use an import lock which isn't likely to go away). For the long-term (or alternate CAPI implementations) you could add no-op shims.

ThePy_MOD_MULTIPLE_INTERPRETERS_NOT_SUPPORTED flag will only allow loading in the main interpreter; that's redundant if you're limited to one module per process.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

However, this causes test_reloading.py::test_full_reimport() to fail. Is that an acceptable trade-off?

I am not opposed to breaking the test/changing the behavior. The ask would be to summarize how things change for:

  • Potentialsubinterpreter users (Ithink that just doesn't work at all anymore, but not sure).
  • Users forimportlib.reload(), currently they get a warning that this is a bad idea. A full-blown error may not be so bad, though.

@AA-TurnerAA-Turnerforce-pushed themulti-phase/umath_linalg branch from59ec898 to6876e24CompareMay 22, 2025 07:01
@AA-Turner
Copy link
Author

Please work on a single PR to reduce maintainer review burden. Once we are settled that we wish to make this change, we can revisit the other ones.

Will do. For context, there isa proposal to deprecate support for single-phase initialisation in favour of multi-phase. We're working toimprove the documentation here too.

A

@AA-Turner
Copy link
Author

Does this impact performance, either import time or runtime?

Local timings usingpython -Ximporttime -c 'import numpy.linalg._umath_linalg' 2>&1| grep _umath_linalg from an editable installation ofnumpy.

Multi-phaseSingle-phase
Mean time (_umath_linalg)5.54ms5.06ms
Mean time (total)4396ms3863ms
Proportion0.126%0.131%

No significant impact of multi-phase on import time. I will investigate running the benchmarks (my laptop is ill-suited to it, though), but there should be no real impact -- this only changes the import process, the end result (initialised module) should be identical performance-wise.

A

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@sebergsebergseberg left review comments

@encukouencukouencukou left review comments

@mattipmattipmattip left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@AA-Turner@seberg@encukou@mattip

[8]ページ先頭

©2009-2025 Movatter.jp