Problem 1541 - [G4GenericTrap] Bug in InsidePolygone method
Summary: [G4GenericTrap] Bug in InsidePolygone method
Status: RESOLVED FIXED
Alias: None
Product: Geant4
Classification: Unclassified
Component: geometry/solids (show other problems)
Version: 9.6
Hardware: All All
: P3 normal
Assignee: tatiana.nikitina
URL:
Depends on:
Blocks:
 
Reported: 2013-12-13 16:25 CET by Nicola Mori
Modified: 2013-12-18 08:52 CET (History)
1 user (show)

See Also:


Attachments
Proposed patch (1012 bytes, patch)
2013-12-13 16:25 CET, Nicola Mori
Details | Diff
Fix for reported problem (60.92 KB, application/octet-stream)
2013-12-17 16:12 CET, tatiana.nikitina
Details

Note You need to log in before you can comment on or make changes to this problem.
Description Nicola Mori 2013-12-13 16:25:37 CET
Created attachment 244 [details]
Proposed patch

I think I discovered a bug in the G4GenericTrap::InsidePolygon routine in Geant4 9.6.p01. In G4GenericTrap.cc at line 247, the "cross" variable is computed: in my understanding, cross is ~ 0 when the point lies on the line defined by the two extrema of the side of the polygon. At line 253 the condition cross~0 is used to check if the point lies on the segment defining a side of the polygon, but this is not correct since cross~0 also when a point lies on the segment but is not within the two extrema. 

As a trivial example, consider the segment defined by the two points (x1,y1)=(-1,0) and (x2,y2)=(1,0) as the side of the polygon lying on the X axis. The point (x,y)=(2,0) lies on the X axis and will give cross=0:

   cross = (x-x1)*(y2-y1) - (y-y1)(x2-x1) = 0

This will trigger the surface check at line 253, which will end returning kSurface at line 275.

I put together an idiotic patch which seems to fix this issue, at least for my use case; I attach it to this bug report.
Comment 1 tatiana.nikitina 2013-12-15 22:36:41 CET
Dear Nicola,

Thank you very much for reporting this problem.

Variable 'test' is used to test, if given point p is Inside segment.
But in case of two end-points of the segment with y1=0 and y2=0  it will fail.

Your proposed solution is good, but it also possible to make special case for y1=y2.

Fix will be tested and included in next Geant4 release.

Best Regards,
Tatiana.
Comment 2 Nicola Mori 2013-12-15 22:44:21 CET
Thanks Tatiana. I didn't test the patch very thoroughly and I think it can be improved (for example, by taking into account some tolerance parameter in the geometric check). Feel free to do whatever you want with it, including throwing it away if you find a better fix.
Comment 3 tatiana.nikitina 2013-12-17 16:12:30 CET
Created attachment 245 [details]
Fix for reported problem

Fix for reported problem
Comment 4 tatiana.nikitina 2013-12-17 16:14:31 CET
Dear Nicola, 

Thank you for the help.
Fix, based on your proposal, is tested and will be included in next Geant4 release.

Best Regards,
Tatiana.
Comment 5 Nicola Mori 2013-12-17 16:18:57 CET
Thank you very much Tatiana. Will the fix appear also in an eventual 9.6.p03 release or only in the Geant4 10 series?
Comment 6 tatiana.nikitina 2013-12-18 08:52:30 CET
Dear Nicola,

Thank you for your message,
Yes, if there will be a patch, fix will be included.

Best Regards,
Tatiana.