sfstoolbox / sfs-python

SFS Toolbox for Python
https://sfs-python.readthedocs.io
MIT License
65 stars 19 forks source link

Hide warnings for a few undefined points #139

Closed hagenw closed 5 years ago

hagenw commented 5 years ago

This suppresses the warnings mentioned in #107.

But I think the current solution is very ugly and covers only a few of the places where this can occur. The problem is that a lot of our sources, e.g. a point source have some points where they are not defined and it is valid to have NaN, Inf, divide by zero or other stuff. But it will be a huge effort to find every line where this has to be fixed.

Another solution would be to generally allow for division by zero or multiplication by NaN in the whole toolbox, but this is also not nice.

Has somebody an idea how to better handle undefined points in numpy?

mgeier commented 5 years ago

Has somebody an idea how to better handle undefined points in numpy?

No, I think allowing NaN and +-Inf in those cases makes sense. Another possibility would be to use "masked arrays", but I don't think those are worth the added complexity.

But I think the current solution is very ugly and covers only a few of the places where this can occur.

I don't think that's a problem as long as the resulting NaN and +-Inf values are theoretically meaningful. There is no need to cover all cases now, we can just keep adding those "ignore" things as we discover new warnings.

generally allow for division by zero or multiplication by NaN in the whole toolbox

We shouldn't do that. There are cases where these warnings show an actual bug or mistake. We should only silence the warnings for situations where we know that the resulting values make sense based on the theory.

mgeier commented 5 years ago

Thanks for the changes!

It looks like invalid='ignore' is still needed:


>>> import sfs
>>> sfs.fd.source.point(100, [6, 6, 6], [6, 6, 6])
[...]/sfs/fd/source.py:88: RuntimeWarning: invalid value encountered in cdouble_scalars
  return numerator / r
(inf+nanj)
>>> 
hagenw commented 5 years ago

Mh, strange for the notebook example I got no warning. I will add it back.