[cctbxbb] normalize() is not safe

[Paul Adams]

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 <PAfonine at lbl.gov> 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, Graeme.Winter at diamond.ac.uk 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: cctbxbb-bounces at phenix-online.org [cctbxbb-bounces at phenix-online.org] on behalf of Oleg Sobolev [osobolev at lbl.gov]
>> 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
> cctbxbb at phenix-online.org
> 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 [ LBenvenue at lbl.gov ][ 1-510-495-2506 ]
--