Skip to content

BUG: reduce using SSE only warns if inside SSE loop#11043

Merged
charris merged 1 commit intonumpy:masterfrom
mattip:fix-11029
May 11, 2018
Merged

BUG: reduce using SSE only warns if inside SSE loop#11043
charris merged 1 commit intonumpy:masterfrom
mattip:fix-11029

Conversation

@mattip
Copy link
Member

@mattip mattip commented May 4, 2018

noticed in discussion on #10370

This is a continuation of PR #9020. Note that if/when PR #11036 is accepted and changes the interface for npy_set_floatstatus*() this PR will need to be updated

Edit: does not affect #11029 , it seems so far there is no relation

@mattip
Copy link
Member Author

mattip commented May 4, 2018

also handles case where NPY_HAVE_SSE2_INTRINSICS is not defined at all

Copy link
Contributor

Choose a reason for hiding this comment

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

this test should probably be in the testminmax of numpy/core/tests/test_umath.py

and we have to hope it fpu exceptions work on all platforms properly

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed.

One of the CI test platforms is a VM without SSE, so passing this test there means this will warn whenever there is a NAN in the output of a min/max reduce. Note the code in loop.c.src to check that condition.

Copy link
Member

Choose a reason for hiding this comment

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

You could use the diagflat trick here, maybe with different values of n.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe let n range through various powers of two, say 8, 16, 32.

Copy link
Member Author

Choose a reason for hiding this comment

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

the test is meant to check that a warning is raised for any single NAN no matter where in a vector, the daigflat call would only emit one warning, not n warnings, no?

fixed to let n range through powers of 2

Copy link
Member

Choose a reason for hiding this comment

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

You can run it with an iterator

for row in diagflat(...):
    whatever

@mattip
Copy link
Member Author

mattip commented May 5, 2018

fixed from reviews, squashed to single commit

mattip added a commit to mattip/numpy that referenced this pull request May 6, 2018
@mattip
Copy link
Member Author

mattip commented May 6, 2018

MSVC for python < 3.6 gives the wrong result, so it seems PR #11036 is a prerequisite to this one.

Edit: link to PR, not issue

charris pushed a commit that referenced this pull request May 10, 2018
…1036)

* BUG: optimizing compilers can reorder call to npy_get_floatstatus

* alternative fix for npy_get_floatstatus, npy_clear_floatstatus

* unify test with pr #11043

* use barrier form of functions in place of PyUFunc_{get,clear}fperr

* update doc, prevent segfault

* MAINT: Do some rewrite on the 1.15.0 release notes.

[ci skip]
@charris
Copy link
Member

charris commented May 10, 2018

#11036 is in.

@mattip
Copy link
Member Author

mattip commented May 10, 2018

rebased and updated

@charris charris merged commit cc8cef9 into numpy:master May 11, 2018
@charris
Copy link
Member

charris commented May 11, 2018

Thanks Matti.

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.

3 participants