includes cup array PType307 #178
Conversation
|
Test data upon request (send them already to @TomTomRixRix). |
There was a problem hiding this comment.
I've left some inline comments within the code mainly to improve the code quality.
Do you have some sort of testing script for reconstructing 3D data so that I can test the cup array without writing it myself? Or shall I do this to avoid biases during testing?
I have one more general question: Is the idea of this detection geometry to have a generic cup array geometry or to have one specific cup array geometry? Currently the code is not clear about this.
- In the case of a specific geometry I would suggest to name the class more specific and perhaps to also hard-code the parameters instead of making them variable in the constructor.
- In case you want to provide a generic cup array geometry, I would remove all hard-coded parameters and assumptions (for convenience you might use them as default values). Most importantly it should then be possible to specify the path from where the detector positions are read, e.g. by setting the path in the constructor or via a setter function.
The ideal case would of course be that we have both, a generic cup array geometry and a specific definition for the cup array geometry which uses the generic definition but fits our test data.
| self.field_of_view_extent_mm = field_of_view_extent_mm | ||
|
|
||
| def check_settings_prerequisites(self, global_settings) -> bool: | ||
| pass |
There was a problem hiding this comment.
Like in the other detection geometries we could also check here if the volume size is large enough to encompass this device and log an error if this is not the case. For example, we could determine the width and length of the device and see if this is smaller than the corresponding volume dimensions
| pass | ||
|
|
||
| def get_detector_element_positions_base_mm(self) -> np.ndarray: | ||
| py_file_path = os.path.dirname(os.path.realpath(__file__)).replace("\\", "/") |
There was a problem hiding this comment.
Why do you need to replace \\ with /? Is this a windows path issue? We should find a solution that works independently from the operating system.
There was a problem hiding this comment.
As there are not backward slashes in Linux paths this line still works fine, because nothing will be replaced as everything is already correct. Nevertheless, I'm not sure if it is the best idea to search for a relative path from this file instead of from the simpa directory.
| for dim in range(3): | ||
| detector_orientations[:, dim] = detector_orientations[:, dim] / norm |
There was a problem hiding this comment.
You can achieve the same result much cleaner and probably also faster with the following code
detector_orientations /= norm[:,None]| self.pitch_mm = pitch_mm | ||
| self.radius_mm = radius_mm | ||
| self.angular_origin_offset = angular_origin_offset |
There was a problem hiding this comment.
If you don't need these variables than there is no need to define them, I guess. The super constructor of the detection geometry has all relevant variables a detection geometry requires and the cup array doesn't seem to need the pitch, radius and angular origin offset so I would leave them out
|
|
||
| def get_detector_element_positions_base_mm(self) -> np.ndarray: | ||
| py_file_path = os.path.dirname(os.path.realpath(__file__)).replace("\\", "/") | ||
| detector_positions = np.load(f'{py_file_path}/cup_positions_PType307', allow_pickle=True) * 1000 # m -> mm |
There was a problem hiding this comment.
Is there a reason, why the detector positions are saved in a pickled file? Wouldn't it be cool if it was saved in a human readable format?
No description provided.