Skip to content

BUG: Removed RuntimeWarning when dividing Polynomial([inf]) by a float, Regarding Issue #21358 #21980

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
Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

BUG: Removed RuntimeWarning when dividing Polynomial([inf]) by a float, Regarding Issue #21358 #21980

wants to merge 9 commits into from

Conversation

ShehanAT
Copy link

Description

This fix removes the RuntimeWarning that appears as a result of dividing Polynomial([inf]) by a float. For example:

import numpy as np
from numpy.polynomial import Polynomial

Polynomial([np.inf]) / 2

Produces the following warning:

.../python3.9/site-packages/numpy/polynomial/polynomial.py:410: RuntimeWarning: invalid value encountered in multiply
  return c1/c2[-1], c1[:1]*0

The fix consists of importing the warnings library and using filterwarnings('ignore') to suppress the RuntimeWarning, like so:

import warnings
warnings.filterwarnings('ignore')

Fixes #21358

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

I've ran the all the test files in the numpy/polynomial/tests folder with all of them passing.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@ShehanAT ShehanAT changed the title BUG: Removed RuntimeWarning when Polynomial([inf]) divided by a float BUG: Removed RuntimeWarning when dividing Polynomial([inf]) by a float, Regarding Issue #21343 Jul 14, 2022
@ShehanAT ShehanAT changed the title BUG: Removed RuntimeWarning when dividing Polynomial([inf]) by a float, Regarding Issue #21343 BUG: Removed RuntimeWarning when dividing Polynomial([inf]) by a float, Regarding Issue #21358 Jul 14, 2022
tylerjereddy
tylerjereddy reviewed Jul 14, 2022
View reviewed changes
rossbar
rossbar reviewed Jul 16, 2022
View reviewed changes
Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

I don't think we'd want to introduce numpy.testing in non-testing code. In fact I don't think warnings filtering is the right way to go here, as I don't think it's thread-safe.

Instead, maybe there's a way to reorganize the operation to avoid the differing scalar/array behavior.

@tylerjereddy
Copy link
Contributor

I don't think we'd want to introduce numpy.testing in non-testing code.

You may want to open an issue to remove the usage in numpy/lib/recfunctions.py that I cited then.

I don't think it's thread-safe.

that's true

@tylerjereddy
Copy link
Contributor

This PR still has left over prints and imports as well.

@tylerjereddy
Copy link
Contributor

I opened gh-21991

@ShehanAT
Copy link
Author

ShehanAT commented Jul 17, 2022 •
edited
Loading

Removed redundant code & lines from numpy/random/_generator.pyx and numpy/polynomial/tests/test_classes.py now.

As @rossbar doesn't recommend the current fix, can you suggest an alternative to using suppress_warnings()?

@rossbar
Copy link
Contributor

rossbar commented Jul 18, 2022

can you suggest an alternative to using suppress_warnings()?

Perhaps taking a closer look at __floordiv__? The warning is being raised in the branch of _div where the remainder is calculated. __floordiv__ doesn't return the remainder anyways, so maybe there's a way to reorganize things so that the remainder isn't computed at all; preferably without adding too much special-casing or code duplication.

@seberg
Copy link
Member

seberg commented Jul 18, 2022

The best solution is to not ignore any warning, since the warning could probably be correct in certain cases (or argue that "invalid value" cannot be). The correct way to hide floating point warnings/errors would be using np.errstate otherwise. (That should be thread-safe, at least in theory; in practice it should also get some fixups using contextvar now – there is a github issue for that.)

@ShehanAT
Copy link
Author

I've replaced suppress_warnings() with np.errstate(all='ignore') now:

with np.errstate(all='ignore'):
            return c1/c2[-1], c1[:1]*0

Let me know what you think.

@seberg
Copy link
Member

seberg commented Jul 20, 2022

@ShehanAT errstate is better than warning filtering, but as also Ross said, this still silences warnings broadly (which can mean silencing correct warnings). Yes, I mentioned errstate, although mainly to teach about it rather than suggest it is the correct solution here.

It would be good to look into avoiding the warning, otherwise it is never an ideal solution and requires a trade-off.

@charris
Copy link
Member

charris commented Aug 8, 2022

maybe there's a way to reorganize things so that the remainder isn't computed at all

The two are computed together so that they are compatible. Computing them separately led to problems, i.e., floor_divide + remainder didn't give the original number..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rossbar rossbar rossbar left review comments

@tylerjereddy tylerjereddy tylerjereddy left review comments

Assignees
No one assigned
Labels
00 - Bug component: numpy.polynomial
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BUG: Polynomial([inf]) divided by a float emits RuntimeWarning: invalid value encountered in multiply
5 participants
@ShehanAT @tylerjereddy @rossbar @seberg @charris