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

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

Merged
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.

@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.

@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?

@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.

@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
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
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

🎉

rgommers added a commit to rgommers/numpy that referenced this pull request Jan 24, 2022
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 pushed a commit to charris/numpy that referenced this pull request Jan 27, 2022
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
03 - Maintenance C++ Related to introduction and use of C++ in the NumPy code base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet