from scitbx import matrix b = matrix.col([0,0,0]) b.normalize() Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/net/anaconda/raid1/olegs/phenix_queue/modules/cctbx_project/scitbx/matrix/__init__.py",
Dear colleagues, I recently discovered that normalize() function of scitbx::vec3<double> is not safe, i.e. this case scitbx::vec3<double> a(0,0,0); a.normalize(); it will produce division by zero and fail. If function contains this being called from python the traceback would be rather vague: <...> Floating-point error (Python and libc call stacks above) This crash may be due to a problem in any imported Python module, including modules which are not part of the cctbx project. To disable the traps leading to this message, define these environment variables (e.g. assign the value 1): BOOST_ADAPTBX_FPE_DEFAULT BOOST_ADAPTBX_SIGNALS_DEFAULT This will NOT solve the problem, just mask it, but may allow you to proceed in case it is not critical. The code that generates this error sits in scitbx/vec3.h, lines 143-153 with a comment that this was done intentionally and a pointer to an appropriate function to do safe normalization. This function is not wrapped to be available in python directly. Similar functionality (but coded in a different place) accessible via python by: line 270, in normalize return self / abs(self) File "/net/anaconda/raid1/olegs/phenix_queue/modules/cctbx_project/scitbx/matrix/__init__.py", line 158, in __truediv__ return rec([e/other for e in self.elems], self.n) ZeroDivisionError: float division by zero Fast grepping shows that normalize() is widely used in xfel, smtbx, rstbx, dxtbx, mmtbx/hydrogens, scitbx/rigid_body, and in some other places. My questions to CCTBX community are: 1. Were you aware of this issue? 2. Should we do something about it: - make the function more safe and slow; - leave it like it is and let everybody know about the issue and let developers deal with it individually; 3. Any other input on the matter? Best regards, Oleg Sobolev.
Dear Oleg philosophy question: what do you think is the correct result for (0,0,0).normalize()? If |x|=0 I would probably expect a /0 error from this - is this your proposal below? I would say that this has not been a problem so far and is used extensively so the current behaviour is probably OK... but that is one opinion of many Documenting this is a good idea though! Best wishes Graeme ________________________________ From: [email protected] [[email protected]] on behalf of Oleg Sobolev [[email protected]] Sent: 01 May 2017 19:10 To: cctbx mailing list Subject: [cctbxbb] normalize() is not safe Dear colleagues, I recently discovered that normalize() function of scitbx::vec3<double> is not safe, i.e. this case scitbx::vec3<double> a(0,0,0); a.normalize(); it will produce division by zero and fail. If function contains this being called from python the traceback would be rather vague: <...> Floating-point error (Python and libc call stacks above) This crash may be due to a problem in any imported Python module, including modules which are not part of the cctbx project. To disable the traps leading to this message, define these environment variables (e.g. assign the value 1): BOOST_ADAPTBX_FPE_DEFAULT BOOST_ADAPTBX_SIGNALS_DEFAULT This will NOT solve the problem, just mask it, but may allow you to proceed in case it is not critical. The code that generates this error sits in scitbx/vec3.h, lines 143-153 with a comment that this was done intentionally and a pointer to an appropriate function to do safe normalization. This function is not wrapped to be available in python directly. Similar functionality (but coded in a different place) accessible via python by:
from scitbx import matrix b = matrix.col([0,0,0]) b.normalize() Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/net/anaconda/raid1/olegs/phenix_queue/modules/cctbx_project/scitbx/matrix/__init__.py", line 270, in normalize return self / abs(self) File "/net/anaconda/raid1/olegs/phenix_queue/modules/cctbx_project/scitbx/matrix/__init__.py", line 158, in __truediv__ return rec([e/other for e in self.elems], self.n) ZeroDivisionError: float division by zero
Fast grepping shows that normalize() is widely used in xfel, smtbx, rstbx, dxtbx, mmtbx/hydrogens, scitbx/rigid_body, and in some other places. My questions to CCTBX community are: 1. Were you aware of this issue? 2. Should we do something about it: - make the function more safe and slow; - leave it like it is and let everybody know about the issue and let developers deal with it individually; 3. Any other input on the matter? Best regards, Oleg Sobolev. -- This e-mail and any attachments may contain confidential, copyright and or privileged material, and are for the use of the intended addressee only. If you are not the intended addressee or an authorised recipient of the addressee please notify us of receipt by returning the e-mail and do not use, copy, retain, distribute or disclose the information in or attached to the e-mail. Any opinions expressed within this e-mail are those of the individual and not necessarily of Diamond Light Source Ltd. Diamond Light Source Ltd. cannot guarantee that this e-mail or any attachments are free from viruses and we cannot accept liability for any damage which you may sustain as a result of software viruses which may be transmitted in or with the message. Diamond Light Source Limited (company no. 4375679). Registered in England and Wales with its registered office at Diamond House, Harwell Science and Innovation Campus, Didcot, Oxfordshire, OX11 0DE, United Kingdom
If memory serves norm of a zero vector is zero, isn't it? So naively I'd expect that function to return zero, not fail in obscure way.. Broadly speaking, norm is length. Length of a zero vector is zero. So my proposal would be to treat zero vectors as special case and do the right thing for it. Pavel On 5/1/17 22:18, [email protected] wrote:
Dear Oleg
philosophy question: what do you think is the correct result for (0,0,0).normalize()?
If |x|=0 I would probably expect a /0 error from this - is this your proposal below?
I would say that this has not been a problem so far and is used extensively so the current behaviour is probably OK... but that is one opinion of many
Documenting this is a good idea though!
Best wishes Graeme
________________________________ From: [email protected] [[email protected]] on behalf of Oleg Sobolev [[email protected]] Sent: 01 May 2017 19:10 To: cctbx mailing list Subject: [cctbxbb] normalize() is not safe
Dear colleagues,
I recently discovered that normalize() function of scitbx::vec3<double> is not safe, i.e. this case scitbx::vec3<double> a(0,0,0); a.normalize(); it will produce division by zero and fail. If function contains this being called from python the traceback would be rather vague: <...> Floating-point error (Python and libc call stacks above) This crash may be due to a problem in any imported Python module, including modules which are not part of the cctbx project. To disable the traps leading to this message, define these environment variables (e.g. assign the value 1): BOOST_ADAPTBX_FPE_DEFAULT BOOST_ADAPTBX_SIGNALS_DEFAULT This will NOT solve the problem, just mask it, but may allow you to proceed in case it is not critical.
The code that generates this error sits in scitbx/vec3.h, lines 143-153 with a comment that this was done intentionally and a pointer to an appropriate function to do safe normalization. This function is not wrapped to be available in python directly.
Similar functionality (but coded in a different place) accessible via python by:
from scitbx import matrix b = matrix.col([0,0,0]) b.normalize() Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/net/anaconda/raid1/olegs/phenix_queue/modules/cctbx_project/scitbx/matrix/__init__.py", line 270, in normalize return self / abs(self) File "/net/anaconda/raid1/olegs/phenix_queue/modules/cctbx_project/scitbx/matrix/__init__.py", line 158, in __truediv__ return rec([e/other for e in self.elems], self.n) ZeroDivisionError: float division by zero
Fast grepping shows that normalize() is widely used in xfel, smtbx, rstbx, dxtbx, mmtbx/hydrogens, scitbx/rigid_body, and in some other places.
My questions to CCTBX community are: 1. Were you aware of this issue? 2. Should we do something about it: - make the function more safe and slow; - leave it like it is and let everybody know about the issue and let developers deal with it individually; 3. Any other input on the matter?
Best regards, Oleg Sobolev.
Hi Pavel
The question was the complement of this i.e. if I ask to normalize a vector v then I return
v/|v|
If |v| == 0 then we do have 0/0 happiness
I am certain that (0,0,0).length() will return 0
http://stackoverflow.com/questions/722073/how-do-you-normalize-a-zero-vector
In *almost every useful case* if you ask to normalize a 0 vector you probably did not want a 0 vector in the first place, else you would have tested this, so any error (even if obscure) is a useful pointer
Cheers Graeme
On 2 May 2017, at 06:44, Pavel Afonine
I agree with Pavel on this. The norm of a zero length vector should be zero (although there is some debate of this). However, it might need to be flagged in some way as Pavel suggests. In most cases when this is passed as an argument there is probably something else going on that isn’t so good.
On May 1, 2017, at 10:44 PM, Pavel Afonine
wrote: If memory serves norm of a zero vector is zero, isn't it? So naively I'd expect that function to return zero, not fail in obscure way..
Broadly speaking, norm is length. Length of a zero vector is zero.
So my proposal would be to treat zero vectors as special case and do the right thing for it.
Pavel
On 5/1/17 22:18, [email protected] wrote:
Dear Oleg
philosophy question: what do you think is the correct result for (0,0,0).normalize()?
If |x|=0 I would probably expect a /0 error from this - is this your proposal below?
I would say that this has not been a problem so far and is used extensively so the current behaviour is probably OK... but that is one opinion of many
Documenting this is a good idea though!
Best wishes Graeme
________________________________ From: [email protected] [[email protected]] on behalf of Oleg Sobolev [[email protected]] Sent: 01 May 2017 19:10 To: cctbx mailing list Subject: [cctbxbb] normalize() is not safe
Dear colleagues,
I recently discovered that normalize() function of scitbx::vec3<double> is not safe, i.e. this case scitbx::vec3<double> a(0,0,0); a.normalize(); it will produce division by zero and fail. If function contains this being called from python the traceback would be rather vague: <...> Floating-point error (Python and libc call stacks above) This crash may be due to a problem in any imported Python module, including modules which are not part of the cctbx project. To disable the traps leading to this message, define these environment variables (e.g. assign the value 1): BOOST_ADAPTBX_FPE_DEFAULT BOOST_ADAPTBX_SIGNALS_DEFAULT This will NOT solve the problem, just mask it, but may allow you to proceed in case it is not critical.
The code that generates this error sits in scitbx/vec3.h, lines 143-153 with a comment that this was done intentionally and a pointer to an appropriate function to do safe normalization. This function is not wrapped to be available in python directly.
Similar functionality (but coded in a different place) accessible via python by:
from scitbx import matrix b = matrix.col([0,0,0]) b.normalize() Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/net/anaconda/raid1/olegs/phenix_queue/modules/cctbx_project/scitbx/matrix/__init__.py", line 270, in normalize return self / abs(self) File "/net/anaconda/raid1/olegs/phenix_queue/modules/cctbx_project/scitbx/matrix/__init__.py", line 158, in __truediv__ return rec([e/other for e in self.elems], self.n) ZeroDivisionError: float division by zero
Fast grepping shows that normalize() is widely used in xfel, smtbx, rstbx, dxtbx, mmtbx/hydrogens, scitbx/rigid_body, and in some other places.
My questions to CCTBX community are: 1. Were you aware of this issue? 2. Should we do something about it: - make the function more safe and slow; - leave it like it is and let everybody know about the issue and let developers deal with it individually; 3. Any other input on the matter?
Best regards, Oleg Sobolev.
_______________________________________________ cctbxbb mailing list [email protected] http://phenix-online.org/mailman/listinfo/cctbxbb
-- Paul Adams Division Director, Molecular Biophysics & Integrated Bioimaging, Lawrence Berkeley Lab Division Deputy for Biosciences, Advanced Light Source, Lawrence Berkeley Lab Adjunct Professor, Department of Bioengineering, U.C. Berkeley Vice President for Technology, the Joint BioEnergy Institute Laboratory Research Manager, ENIGMA Science Focus Area Building 33, Room 347 Building 80, Room 247 Building 978, Room 4126 Tel: 1-510-486-4225, Fax: 1-510-486-5909 http://cci.lbl.gov/paul Lawrence Berkeley Laboratory 1 Cyclotron Road BLDG 33R0345 Berkeley, CA 94720, USA. Executive Assistant: Louise Benvenue [ [email protected] ][ 1-510-495-2506 ] --
Hi,
On 2 May 2017, at 07:44, Pavel Afonine
wrote: If memory serves norm of a zero vector is zero, isn't it? So naively I'd expect that function to return zero, not fail in obscure way..
Broadly speaking, norm is length. Length of a zero vector is zero.
So my proposal would be to treat zero vectors as special case and do the right thing for it.
If the current implementation vec3 normalize() const { return (*this) / length(); } was to be replaced by the following, as Pavel proposed if I understand correctly, if(length() == 0) raise scitbx::error(“a zero vector cannot be normalised”); return (*this)/length(); this may not result in any performance hit in fact. Indeed the compiler would still be allowed to inline normalize and then the processor branch prediction unit may realise the branch throwing the exception is hardly ever taken, and take advantage of that to reorder calculations to, in effect, make the code equivalent to the current implementation. But this is the point: the current implementation is guaranteed to be optimal whether the rewrite may be optimal. So if this function appears in a critical path, you would need to check there is no performance hit. Best wishes, Luc
participants (5)
-
Graeme.Winter@diamond.ac.uk
-
Luc Bourhis
-
Oleg Sobolev
-
Paul Adams
-
Pavel Afonine