Skip to content

MAINT: Replace numpy custom generation engine by raw C++ #19713

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 22, 2021
Merged

MAINT: Replace numpy custom generation engine by raw C++ #19713

merged 1 commit into from
Oct 22, 2021

Conversation

serge-sans-paille
Copy link
Contributor

This is just a technical prototype to measure and discuss the impact and
implication of moving to C++ for kernel code generation.

@serge-sans-paille serge-sans-paille force-pushed the feature/to-cxx-and-beyond branch from 848d64f to bafb152 Compare August 19, 2021 07:51
@eric-wieser
Copy link
Member

This looks great to me, but I don't know how easy it will be to get this working in our build system.

@serge-sans-paille serge-sans-paille force-pushed the feature/to-cxx-and-beyond branch from bafb152 to a0a48ce Compare August 19, 2021 12:30
@rgommers rgommers added the C++ Related to introduction and use of C++ in the NumPy code base label Aug 19, 2021
@rgommers
Copy link
Member

Interesting, thanks @serge-sans-paille!

I don't know how easy it will be to get this working in our build system.

It looks like it builds okay on Azure, there are just a few failures. On TravisCI I see:

cc1plus: warning: ‘-Werror=’ argument ‘-Werror=implicit-function-declaration’ is not valid for C++

-Werror is added via runtests.py --warn-error in CI only, should be easy to fix.

Not sure what the exact problem is in cygwin_build_test. I had totally missed that we have a Cygwin CI job now.

Do you expect specific issues @eric-wieser?

@serge-sans-paille serge-sans-paille force-pushed the feature/to-cxx-and-beyond branch from a0a48ce to 5016080 Compare August 19, 2021 13:34
@eric-wieser
Copy link
Member

Do you expect specific issues @eric-wieser?

No, but I think I've seen build/distribution-issue-based resistance to switching to c++ before, and am merely expecting someone else to bring those up again. I don't know the details I'm afraid.

@rgommers
Copy link
Member

Okay thanks. I think there are likely to be some fiddly details, but given that numpy.distutils supports C++ just fine and it works for SciPy, I would not expect any really fundamental problems.

@serge-sans-paille serge-sans-paille force-pushed the feature/to-cxx-and-beyond branch from 5016080 to a6982ad Compare August 19, 2021 14:19
@charris
Copy link
Member

charris commented Aug 19, 2021

and am merely expecting someone else to bring those up again.

We discussed that in one of the triage meetings and decided modern C++ was acceptable, @seiko2plus wanted it.

@serge-sans-paille serge-sans-paille force-pushed the feature/to-cxx-and-beyond branch 4 times, most recently from 5cd48a6 to d401263 Compare August 20, 2021 07:31
@serge-sans-paille
Copy link
Contributor Author

All tests green except cygwin, and there's no error log so I can't track the error correctly, if some knows more than me about it ?

@serge-sans-paille serge-sans-paille force-pushed the feature/to-cxx-and-beyond branch 14 times, most recently from 7dbc175 to a9d5772 Compare August 21, 2021 20:33
@serge-sans-paille serge-sans-paille force-pushed the feature/to-cxx-and-beyond branch 8 times, most recently from 3fa3dc5 to 3a59afd Compare October 22, 2021 09:25
@serge-sans-paille
[demo] how-to replacing numpy custom generation engine by raw C++
2ae7aeb 
This is just a technical prototype to measure and discuss the impact and
implication of moving to C++ for kernel code generation.
@serge-sans-paille serge-sans-paille force-pushed the feature/to-cxx-and-beyond branch from 3a59afd to 2ae7aeb Compare October 22, 2021 09:57
@serge-sans-paille
Copy link
Contributor Author

And all green again ;-)

@mattip
Copy link
Member

mattip commented Oct 22, 2021

In the recent developer meeting, we decided to put this in as an experimental enhancement, without actually pushing to migrate the templated code to c++ templates for the 1.22 release. That way we can see if the infrastructure is solid before progressing.

@mattip mattip merged commit 53d2a83 into numpy:main Oct 22, 2021
@mattip
Copy link
Member

mattip commented Oct 22, 2021

Thanks @serge-sans-paille

@charris charris changed the title [demo] how-to replacing numpy custom generation engine by raw C++ MAINT: Replace numpy custom generation engine by raw C++ Oct 23, 2021
@SylvainCorlay
Copy link

🎉

@seiko2plus seiko2plus mentioned this pull request Oct 27, 2021
Merged
@seiko2plus seiko2plus mentioned this pull request Nov 10, 2021
Closed
@serge-sans-paille serge-sans-paille mentioned this pull request Dec 2, 2021
Merged
@aragilar aragilar mentioned this pull request Jan 7, 2022
Closed
rgommers added a commit to rgommers/numpy that referenced this pull request Jan 24, 2022
@rgommers
BUG: distutils: fix building mixed C/Fortran extensions
1e83f83 
In SciPy we had a couple of cases where we build a Python extension
with C source files but linked against static libraries built from
Fortran code. Those should be using the Fortran linker, but this
was broken in 1.22.0 by numpygh-19713 (the PR that introduced C++ in NumPy).

This fixes a few issues in the `build_ext` command, and documents better
what is going on there.

Should close SciPy issues 8325 and 15414.
@rgommers rgommers mentioned this pull request Jan 24, 2022
Merged
charris pushed a commit to charris/numpy that referenced this pull request Jan 27, 2022
@rgommers @charris
BUG: distutils: fix building mixed C/Fortran extensions
da48521 
In SciPy we had a couple of cases where we build a Python extension
with C source files but linked against static libraries built from
Fortran code. Those should be using the Fortran linker, but this
was broken in 1.22.0 by numpygh-19713 (the PR that introduced C++ in NumPy).

This fixes a few issues in the `build_ext` command, and documents better
what is going on there.

Should close SciPy issues 8325 and 15414.
@charris charris mentioned this pull request Jan 27, 2022
Merged
@jschueller jschueller mentioned this pull request Jan 28, 2022
Closed
@serge-sans-paille serge-sans-paille mentioned this pull request Mar 21, 2022
Merged
@mattip mattip mentioned this pull request Apr 5, 2022
Closed
@scratchmex scratchmex mentioned this pull request Aug 25, 2022
Open
@rkern rkern mentioned this pull request Apr 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seberg seberg seberg left review comments

@pearu pearu pearu left review comments

@mattip mattip mattip left review comments

@peterbell10 peterbell10 peterbell10 left review comments

@fawazfzk fawazfzk fawazfzk approved these changes

Assignees
No one assigned
Labels
03 - Maintenance C++ Related to introduction and use of C++ in the NumPy code base
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.