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.

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

Debugging #8544 #8671

Closed
wants to merge 12 commits into from
Closed

Debugging #8544 #8671

wants to merge 12 commits into from

Conversation

stuartarchibald
Copy link
Contributor

As title. Do not merge.

@stuartarchibald
Remove reliance on npy_<impl> ufunc loops.
96f5549 
This removes Numba's reliance on NumPy ufunc inner loops. All the
cases where a ufunc inner loop (or something from
`numpy/npy_math.h`) was present, have been replaced as follows:

* `np.signbit` -> Numba custom builder implementation.
* `np.nextafter` -> Numba helperlib wrapped C library call.
* `np.spacing` -> Numba custom builder implementation.
* `np.log2` -> llvm intrinsic.
* `np.exp2` -> llvm intrinsic.
* `np.logaddexp` -> custom Numba overload translation of NumPy C code.
* `np.logaddexp2` -> custom Numba overload translation of NumPy C code.
* Numba helperlib `numba_cpowf` -> now uses `<complex.h>` functions in
  place of NumPy functions.
@stuartarchibald
Add in MSVC compatible complex C type handling.
71be82f 
As title.
@stuartarchibald
Remove _npymath_exports.c and references to it and its functions.
ca033d3 
As title.
@stuartarchibald
Fix typo and rename complex_float type.
0205988 
As title.
@stuartarchibald
Remove reliance on libnpymath.a static linkage.
995a7d1 
As title.
@stuartarchibald
Respond to feedback RE signature args unpacking.
7966e9b 
As title.
@stuartarchibald
Add test for +/-inf inputs to np.spacing ufunc.
a28cf62 
As title.
@stuartarchibald
Attempt to fix logic for pycc library selection on windows.
ed2ffa1 
As title.
@stuartarchibald
Merge remote-tracking branch 'upstream/main' into wip/remove_npy_ufun…
c12b181 
…c_loop_reliance

Resolved conflicts in:
	numba/tests/test_ufuncs.py
@stuartarchibald
Debugging suspected external compiler tooling writing to stdout.
e3867ca 
As title.
@stuartarchibald stuartarchibald added the abandoned PR is abandoned (no reason required) label Dec 15, 2022
stuartarchibald
stuartarchibald commented Dec 15, 2022
View reviewed changes
numba/pycc/platform.py Outdated
@@ -58,10 +58,10 @@ def _check_external_compiler():
ntf.close()
# *output_dir* is set to avoid the compiler putting temp files
# in the current directory.
compiler.compile([ntf.name], output_dir=Path(ntf.name).anchor)
#compiler.compile([ntf.name], output_dir=Path(ntf.name).anchor)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Potential problem line.

@stuartarchibald
Copy link
Contributor Author

ebc9299 has no temp.{c, cxx} in the log, which suggests this line #8671 (comment) is potentially the cause.

@stuartarchibald
Copy link
Contributor Author

e221307 has no temp.{c, cxx} in the log but does have this line #8671 (comment) present. This perhaps suggests it's NumPy's distutils patching standard distutils that "fixes" the problem. Seems like this could be due to the replacement of .spawn in the CCompiler https://github.com/numpy/numpy/blob/62e075c1c22530528bf5b6778ef51e212f65acb7/numpy/distutils/ccompiler.py#L112-L183.

The stdlib vendored distutils variant of .spawn uses a Popen with no specification for stderr handling (the new process will inherit the file descriptors from the parent): https://github.com/python/cpython/blob/3939a4b7d95a403ee587ff5d222db53ec83bec2c/Lib/distutils/spawn.py#L79 whereas the NumPy patched version of .spawn has redirection to stdout in a checked_output call https://github.com/numpy/numpy/blob/62e075c1c22530528bf5b6778ef51e212f65acb7/numpy/distutils/ccompiler.py#L145. i.e. any output to stdout/stderr from the subprocess in the NumPy patch version will be "hidden".

@stuartarchibald
Copy link
Contributor Author

Closing, investigation is complete.

@stuartarchibald stuartarchibald mentioned this pull request Dec 16, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
abandoned PR is abandoned (no reason required)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@stuartarchibald