Skip to content

Fix Parallel2D system_description precision error#2316

Open
gfardell wants to merge 3 commits into
TomographicImaging:masterfrom
gfardell:bug_fix_ag_parallel2D
Open

Fix Parallel2D system_description precision error#2316
gfardell wants to merge 3 commits into
TomographicImaging:masterfrom
gfardell:bug_fix_ag_parallel2D

Conversation

@gfardell
Copy link
Copy Markdown
Member

@gfardell gfardell commented Apr 29, 2026

Changes

Fixed Parallel2D geometry raising exception in system_description due to near-zero vector when rotation axis and detector positions are close but not equal.

Testing you performed

Please add any demo scripts to https://github.com/TomographicImaging/CIL-Demos/tree/main/misc

Related issues/links

Checklist

  • I have performed a self-review of my code
  • I have added docstrings in line with the guidance in the developer guide
  • I have updated the relevant documentation
  • I have implemented unit tests that cover any new or modified functionality
  • CHANGELOG.md has been updated with any functionality change
  • Request review from all relevant developers

gfardell and others added 2 commits April 29, 2026 17:40
Signed-off-by: Laura Murgatroyd <60604372+lauramurgatroyd@users.noreply.github.com>
Copy link
Copy Markdown
Member

@lauramurgatroyd lauramurgatroyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gfardell, I think we need to change the code.
Please can you expand on what the issue was that you encountered?

Comment on lines +457 to +458
if abs(dot_product) < 1e-8:
return SystemConfiguration.SYSTEM_SIMPLE
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about a case where the detector and rotation axis are offset on the y axis. Shouldn't that also be system_simple?

e.g.

AG = AcquisitionGeometry.create_Parallel2D(detector_position=[0,10], rotation_axis_position=[0,0])
self.assertTrue(AG.system_description=='simple')

currently fails (and I think we should add it as a unit test)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the previous code worked as expected because it checked if all vector values were equal and if not, checked the difference vector was parallel to the ray direction

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. We don't set detector position in many parallel cases so I didn't spot it.

@gfardell
Copy link
Copy Markdown
Member Author

The issue is that https://github.com/gfardell/CIL/blob/a4e2b29eab32aa5fefc11dc02584d762c601729f/Wrappers/Python/cil/framework/acquisition_geometry.py#L52 is returned if the points are equal to a tolerance, that wasn't a tighter tolerance than the code route that called it.

@lauramurgatroyd
Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants