search

SyneRBI / SIRF

Main repository for the CCP SynerBI software
http://www.ccpsynerbi.ac.uk
Other
59 stars 29 forks source link

How to handle DataContainer division by zero #865

Closed paskino closed 3 years ago

paskino commented 3 years ago

Currently, not all DataContainer sub classes will throw an error on division by zero, https://github.com/SyneRBI/SIRF/issues/863.

However, this is not standard for other packages, such as numpy for instance which returns numpy.inf.

We should define a policy and develop the code accordingly.

KrisThielemans commented 3 years ago

ah, I was just editing #863 😄 .

Here's my policy: do not do anything special. The hardware will take care of it. On IEEE compatible systems, it will return inf or nan (depending on 1/0 or 0/0). On other systems, I don't know. But it's not our business to take care of this.

paskino commented 3 years ago

So, we need to implement the division in C++ accordingly.

KrisThielemans commented 3 years ago

So, we need to implement the division in C++ accordingly.

Not sure what you mean. I'm saying, "don't do anything special". If you divide in C++, your processor will decide what it does. We just accept that. (In practice, everyone will have an IEEE-compliant CPU. With GPUs it'll be different I guess).

KrisThielemans commented 3 years ago

excellent. might need some changes on the Python side? For instance, @paskino tests on the division throwing I think.

evgueni-ovtchinnikov commented 3 years ago

@paskino: could you fix your Python zero division tests please? I am not sure I understand them, so afraid to damage something.

evgueni-ovtchinnikov commented 3 years ago

@paskino: disabled zero-division checks in Utilities.py for now to pacify Travis