Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

attempt to make CListEventCyl...DiscreteDetectors templated for block… #1209

Closed
wants to merge 1 commit into from

Conversation

danieldeidda
Copy link
Collaborator

…OnCylindrical

This seems to give the same sinogram I was getting before the change. However, when using BlocksOnCylindrical the sinogram obtained is exactly the same. Would we expect this to happen? if yes then pheraps there is no need to template thiese functions. I was expecting some difference.

This is not most elegant way of doing this, I was trying to avoid changes in other classes but there are a few part that I am not very happy about (feedback welcome!)

@KrisThielemans
Copy link
Collaborator

Not sure what you mean when saying that the sinograms are the same. If you pretend a cyl-scanner is a blocks-scanner, and unlist that way, and then fix the header, yes, you would get the same results (as we're just working on indices really).

See also #994 which was another attempt at the same problem

@danieldeidda
Copy link
Collaborator Author

Not sure what you mean when saying that the sinograms are the same. If you pretend a cyl-scanner is a blocks-scanner, and unlist that way, and then fix the header, yes, you would get the same results (as we're just working on indices really).

See also #994 which was another attempt at the same problem

what I meant was that if we try to unlist the data using different functions (according to whether we want Cylindrical or BlocksOnCylindrical) ex: find_cartesian_coordinates_given_scanner_coordinates I would expect that the events are somehow organised differently between the two geometry. But I guess as you said we are only working on indices . I suppose it will make a difference if we get the position and not the index

@KrisThielemans
Copy link
Collaborator

I suppose it will make a difference if we get the position and not the index

definitely

@KrisThielemans
Copy link
Collaborator

These changes are now obsolete after #1222

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants