8000 BUILD: Hide platform configuration probe behind --debug-configure by mattip · Pull Request #14518 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

BUILD: Hide platform configuration probe behind --debug-configure #14518

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

Merged
merged 8 commits into from
Sep 20, 2019

Conversation

mattip
Copy link
Member
@mattip mattip commented Sep 15, 2019

As requested in #14470, this adds a --debug-configure option to runtests.py and setup.py build to hide all the compiler output during the configuration probe to create config.h and _numpyconfig.h.

This also converts some of the direct print to stdout so that it uses the logger instead. The lack of the flag simply raises the log level to WARN from INFO for the duration of the noisy function.

@mattip
Copy link
Member Author
mattip commented Sep 15, 2019

Should I add the --debug-configure flag to the CI runs?

@@ -16,8 +16,8 @@ class build(old_build):
user_options = old_build.user_options + [
('fcompiler=', None,
"specify the Fortran compiler type"),
('parallel=', 'j',
Copy link
Member

Choose a reason for hiding this comment

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

This change seems unrelated, why does parallel need to be removed here and in finalize_options?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is redundant - the base build class has this option (on python 3)

Copy link
Member Author

Choose a reason for hiding this comment

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

Specifically it was added in CPython v3.5.0

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes. We may have to add it back once we figure out why it's buggy right now, but let's remove it till someone gets annoyed enough to start looking into that.

# build_clib cannot handle generate_config_h and generate_numpyconfig_h
# (don't ask). Because clib are generated before extensions, we have to
# explicitly add an extension which has generate_config_h and
# generate_numpyconfig_h as sources *before* adding npymath.
Copy link
Member

Choose a reason for hiding this comment

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

what's going on here, did all this become obsolete somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should have been removed when we merged umath and multiarray. It causes the config file generation to be run twice, which I noticed when working on this PR.

@rgommers
Copy link
Member

Should I add the --debug-configure flag to the CI runs?

Probably a good idea. The SDIST=1 build should use the new default (hide config output), in the other CI builds it's mostly just noise I think. Unless other people have a use for it, I can't remember many such cases ....

@rgommers
Copy link
Member

Thanks for tackling this, will be very nice to have!

@mattip
Copy link
Member Author
mattip commented Sep 15, 2019

Comparing the logs from another PR's travis run to this one, the lines from

building library "npysort" sources to
building extension "numpy.core._multiarray_tests" sources now become from

building library "npysort" sources to
building extension "numpy.core._multiarray_tests" sources, removing about 1600 lines.

@mattip
Copy link
Member Author
mattip commented Sep 16, 2019

failing because of #14526

runtests.py Outdated
@@ -67,6 +67,8 @@ def main(argv):
parser = ArgumentParser(usage=__doc__.lstrip())
parser.add_argument("--verbose", "-v", action="count", default=1,
help="more verbosity")
parser.add_argument("--debug-configure", action="store_true",
help="add -v to build_src to show cconfiguration compiler output")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is pretty vague; can we describe what the actual feature is that is enabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

"adds -v to the build_src subcommand which will show compiler output while probing platform features used to create _numpyconfig.h and config.h" seems a bit too much.

Any ideas what would be more appropriate?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the problem is that there are several things being addressed here: -v is being added to build_src, but only for a certain type of build? Or is it always added? If always, then the latter part of the sentence is not necessary. It can simply be something like "Increase verbosity of build_src by adding -v flag". But the "configure" in --debug-configure makes me think it was intended for something more specific.

Copy link
Member Author

Choose a reason for hiding this comment

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

another option would be to not add it to runtests.py at all (which would always build without the flag), and in CI explicitly do setup.py build build_src -v; runtests.py.

Copy link
Member

Choose a reason for hiding this comment

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

and in CI explicitly do setup.py build build_src -v; runtests.py

That doesn't seem desirable at all, we want to run CI in the ways we expect humans to use things, not some arcane invocation. There's no real issue here, so why change anything?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think just a slightly more descriptive help string would be sufficient.

@rgommers
Copy link
Member

CI failure python3: can't open file 'build': [Errno 2] No such file or directory is related.

I'm -1 on the interface changes and promoting build_src usage, this was good before.

@mattip
Copy link
Member Author
mattip commented Sep 20, 2019

rebased and reverted the change. The help string now reads

...
optional arguments:
  -h, --help            show this help message and exit
  --verbose, -v         more verbosity
  --debug-configure     add -v to build_src to show compiler configuration
                        output while creating _numpyconfig.h and config.h
  --no-build, -n        do not build the project (use system installed
                        version)
...

clarifications happily accepted.

@rgommers
Copy link
Member

That help description seems very clear to me, thanks @mattip.

@rgommers rgommers merged commit 815061c into numpy:master Sep 20, 2019
@rgommers
Copy link
Member

In it goes, very nice to have this!

@rgommers rgommers added this to the 1.18.0 release milestone Sep 20, 2019
@rgommers
Copy link
Member

Hmm, one of the Windows builds on Azure for the merge commit failed, see https://dev.azure.com/numpy/numpy/_build/results?buildId=5577.

Related to gh-14464, which may have a bad interaction with this PR. Any thoughts @mattip?

================================== FAILURES ===================================
____________________ TestMatmul.test_vector_matrix_values _____________________

self = <numpy.core.tests.test_multiarray.TestMatmul object at 0x14B88330>

    def test_vector_matrix_values(self):
        vec = np.array([1, 2])
        mat1 = np.array([[1, 2], [3, 4]])
        mat2 = np.stack([mat1]*2, axis=0)
        tgt1 = np.array([7, 10])
        tgt2 = np.stack([tgt1]*2, axis=0)
        for dt in self.types[1:]:
            v = vec.astype(dt)
            m1 = mat1.astype(dt)
            m2 = mat2.astype(dt)
>           res = self.matmul(v, m1)
E           RuntimeWarning: invalid value encountered in matmul

dt         = 'F'
m1         = array([[1.+0.j, 2.+0.j],
       [3.+0.j, 4.+0.j]], dtype=complex64)
m2         = array([[[1.+0.j, 2.+0.j],
        [3.+0.j, 4.+0.j]],

       [[1.+0.j, 2.+0.j],
        [3.+0.j, 4.+0.j]]], dtype=complex64)
mat1       = array([[1, 2],
       [3, 4]])
mat2       = array([[[1, 2],
        [3, 4]],

       [[1, 2],
        [3, 4]]])
res        = array([[ 7., 10.],
       [ 7., 10.]], dtype=float64)
self       = <numpy.core.tests.test_multiarray.TestMatmul object at 0x14B88330>
tgt1       = array([ 7, 10])
tgt2       = array([[ 7, 10],
       [ 7, 10]])
v          = array([1.+0.j, 2.+0.j], dtype=complex64)
vec        = array([1, 2])

C:\hostedtoolcache\windows\Python\3.6.8\x86\lib\site-packages\numpy\core\tests\test_multiarray.py:6031: RuntimeWarning
____________________ TestMatmul.test_matrix_vector_values _____________________

self = <numpy.core.tests.test_multiarray.TestMatmul object at 0x14C75330>

    def test_matrix_vector_values(self):
        vec = np.array([1, 2])
        mat1 = np.array([[1, 2], [3, 4]])
        mat2 = np.stack([mat1]*2, axis=0)
        tgt1 = np.array([5, 11])
        tgt2 = np.stack([tgt1]*2, axis=0)
        for dt in self.types[1:]:
            v = vec.astype(dt)
            m1 = mat1.astype(dt)
            m2 = mat2.astype(dt)
>           res = self.matmul(m1, v)
E           RuntimeWarning: invalid value encountered in matmul

dt         = 'F'
m1         = array([[1.+0.j, 2.+0.j],
       [3.+0.j, 4.+0.j]], dtype=complex64)
m2         = array([[[1.+0.j, 2.+0.j],
        [3.+0.j, 4.+0.j]],

       [[1.+0.j, 2.+0.j],
        [3.+0.j, 4.+0.j]]], dtype=complex64)
mat1       = array([[1, 2],
       [3, 4]])
mat2       = array([[[1, 2],
        [3, 4]],

       [[1, 2],
        [3, 4]]])
res        = array([[ 5., 11.],
       [ 5., 11.]], dtype=float64)
self       = <numpy.core.tests.test_multiarray.TestMatmul object at 0x14C75330>
tgt1       = array([ 5, 11])
tgt2       = array([[ 5, 11],
       [ 5, 11]])
v          = array([1.+0.j, 2.+0.j], dtype=complex64)
vec        = array([1, 2])

C:\hostedtoolcache\windows\Python\3.6.8\x86\lib\site-packages\numpy\core\tests\test_multiarray.py:6058: RuntimeWarning
________________ TestMatmulOperator.test_vector_matrix_values _________________

self = <numpy.core.tests.test_multiarray.TestMatmulOperator object at 0x14C545B0>

    def test_vector_matrix_values(self):
        vec = np.array([1, 2])
        mat1 = np.array([[1, 2], [3, 4]])
        mat2 = np.stack([mat1]*2, axis=0)
        tgt1 = np.array([7, 10])
        tgt2 = np.stack([tgt1]*2, axis=0)
        for dt in self.types[1:]:
            v = vec.astype(dt)
            m1 = mat1.astype(dt)
            m2 = mat2.astype(dt)
>           res = self.matmul(v, m1)
E           RuntimeWarning: invalid value encountered in matmul

dt         = 'F'
m1         = array([[1.+0.j, 2.+0.j],
       [3.+0.j, 4.+0.j]], dtype=complex64)
m2         = array([[[1.+0.j, 2.+0.j],
        [3.+0.j, 4.+0.j]],

       [[1.+0.j, 2.+0.j],
        [3.+0.j, 4.+0.j]]], dtype=complex64)
mat1       = array([[1, 2],
       [3, 4]])
mat2       = array([[[1, 2],
        [3, 4]],

       [[1, 2],
        [3, 4]]])
res        = array([[ 7., 10.],
       [ 7., 10.]], dtype=float64)
self       = <numpy.core.tests.test_multiarray.TestMatmulOperator object at 0x14C545B0>
tgt1       = array([ 7, 10])
tgt2       = array([[ 7, 10],
       [ 7, 10]])
v          = array([1.+0.j, 2.+0.j], dtype=complex64)
vec        = array([1, 2])

C:\hostedtoolcache\windows\Python\3.6.8\x86\lib\site-packages\numpy\core\tests\test_multiarray.py:6031: RuntimeWarning
________________ TestMatmulOperator.test_matrix_vector_values _________________

self = <numpy.core.tests.test_multiarray.TestMatmulOperator object at 0x14B88470>

    def test_matrix_vector_values(self):
        vec = np.array([1, 2])
        mat1 = np.array([[1, 2], [3, 4]])
        mat2 = np.stack([mat1]*2, axis=0)
        tgt1 = np.array([5, 11])
        tgt2 = np.stack([tgt1]*2, axis=0)
        for dt in self.types[1:]:
            v = vec.astype(dt)
            m1 = mat1.astype(dt)
            m2 = mat2.astype(dt)
>           res = self.matmul(m1, v)
E           RuntimeWarning: invalid value encountered in matmul

dt         = 'F'
m1         = array([[1.+0.j, 2.+0.j],
       [3.+0.j, 4.+0.j]], dtype=complex64)
m2         = array([[[1.+0.j, 2.+0.j],
        [3.+0.j, 4.+0.j]],

       [[1.+0.j, 2.+0.j],
        [3.+0.j, 4.+0.j]]], dtype=complex64)
mat1       = array([[1, 2],
       [3, 4]])
mat2       = array([[[1, 2],
        [3, 4]],

       [[1, 2],
        [3, 4]]])
res        = array([[ 5., 11.],
       [ 5., 11.]], dtype=float64)
self       = <numpy.core.tests.test_multiarray.TestMatmulOperator object at 0x14B88470>
tgt1       = array([ 5, 11])
tgt2       = array([[ 5, 11],
       [ 5, 11]])
v          = array([1.+0.j, 2.+0.j], dtype=complex64)
vec        = array([1, 2])

@mattip
Copy link
Member Author
mattip commented Sep 20, 2019

That is the infamous heisenbug that occurs 1/1000.

@charris
Copy link
Member
charris commented Sep 21, 2019

This broke the appveyor wheel builds. See https://ci.appveyor.com/project/matthew-brett/numpy-wheels/builds/27571654/job/v5tm7ljl6htcfa0a. Probably the changes in setup.py put things out of order.

charris added a commit that referenced this pull request Sep 22, 2019
@mattip mattip deleted the hide-config-probe branch June 8, 2020 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0