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

Numpy errors are supressed by default #177

Open
CaspianChaharom opened this issue Mar 7, 2022 · 5 comments
Open

Numpy errors are supressed by default #177

CaspianChaharom opened this issue Mar 7, 2022 · 5 comments

Comments

@CaspianChaharom
Copy link

Some coordinate systems can not represent some vectors. For example, if adding two vectors in pt,phi,eta,E coordinates that have opposite azimuthal components that add to zero, the resulting longitudinal coordinate eta will be infinity:

import numpy as np
import vector
v1 = vector.obj(pt=1, phi=0,     eta=1, E=1)
v2 = vector.obj(pt=1, phi=np.pi, eta=1, E=1)
new_vector =  v1 + v2  # this vector can not be represented in the pt,phi,eta,E coordinate system

By default, such errors are suppressed in the library

with numpy.errstate(all="ignore"):

Without that line, the default Numpy error would be shown

/usr/lib/python3.10/site-packages/vector/_compute/spatial/eta.py:43: RuntimeWarning: divide by zero encountered in arctanh
  return lib.nan_to_num(lib.arctanh(z / lib.sqrt(rho ** 2 + z ** 2)), nan=0.0)
vector.obj(pt=1.2246467991473532e-16, phi=1.5707963267948966, eta=1.7976931348623157e+308, E=2)

I think it would be better if the user is made aware of the error by default (rather than suppressing by default). Additionally, that leaves the choice of suppressing errors or not to the user, by adding numpy.errstate(all="ignore") in their code.

@jpivarski
Copy link
Member

At least I can say that the reason for this setting is that many of the formulas would raise warnings all the time, due to intermediate states of the calculation, even though the final results are finite (not NaN or Inf). You can see in this one that nan_to_num is used to turn NaN into zero (to follow ROOT's behavior).

The default should probably be to suppress the warnings, but it would be nice to give users control over it. How should that be, a global setting or a context manager? The setting would have to get into every compute submodule.

@CaspianChaharom
Copy link
Author

CaspianChaharom commented Mar 7, 2022

Ok, I see.
I'm not sure how important it is, but the reason I suggested showing the warnings by default may have been specific to my use case. I was adding many vectors from a file with an eta azimuthal component, and getting the mass of the summed vector. It turned out a few of them had zero longitudinal components, leading to infinite masses that were used later on in the script. I ended up having to go through looking at many intermediate results to track down that the issue was an infinite eta, so it would have been nice to have the warning shown when that line was executed.

Both ideas seem ok:

  • let the errors be handled by the underlying library
import numpy as np
import vector
v1 = vector.obj(pt=1, phi=0,     eta=1, E=1)
v2 = vector.obj(pt=1, phi=np.pi, eta=1, E=1)
np.errstate(all="ignore"):  # ignore the error that the following line should generate
    new_vector =  v1 + v2
    print(new_vector.mass)  # this should print -1.23, but due to the coordinate system, will be -inf

Searching the _compute folder I see 79 with numpy.errstate(all="ignore"): that would need to be updated

  • With a global setting
import numpy as np
import vector
vector.show_errors = False
v1 = vector.obj(pt=1, phi=0,     eta=1, E=1)
v2 = vector.obj(pt=1, phi=np.pi, eta=1, E=1)
new_vector =  v1 + v2
print(new_vector.mass)

I'm not sure what is the best way to give users control over it, but if it's the case that many of the formulas would raise warnings all the time1 then the global setting might be better.

Footnotes

  1. Though the RuntimeWarning: divide by zero encountered in arctanh is shown only the first time and not again if encountered again

@jpivarski
Copy link
Member

A third possibility would be to put an isnan check after ignoring all the errors in the intermediate calculations. We'd have to make sure this "plays well" with all the intended backends, and it would be another level of warning that needs to be suppressable.

Also, we'd have to decide whether to call out infinity as being as bad as not a number. I think infinity is more benign than not a number: it appears in cases that—if there were any small errors—would be large numbers instead of infinity, and they do the right things in <, > operations.

Though the RuntimeWarning: divide by zero encountered in arctanh is shown only the first time and not again if encountered again

This is true for all the NumPy warnings, just because that's how warnings work in Python. (For that reason, I don't see them as being very helpful. They tell you that something bad happened, but by not stopping the program and but roasting themselves, they don't tell you where or how many times the bad thing happened. And then, if the bad thing is in an intermediate step and expected, they become useless noise.

On the other hand, these warnings can also be turned into errors, which are useful if they can be suppressed when they're expected.

@CaspianChaharom
Copy link
Author

I think there should at least be an option to have infinity shown as a warning. But I do see the utility you mentioned of not marking infinity as a bad number. Are you thinking of a global verbosity config?
(silent, show all except infinity, show all)

@jpivarski
Copy link
Member

I guess it should have the same or a similar interface to np.errstate, some vector.errstate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants