[CCPPETMR/SIRF] VoxelisedGeometricalInfo should check its fields (#371)

0 wyświetleń
Przejdź do pierwszej nieodczytanej wiadomości

Ashley Gillman

nieprzeczytany,
19 kwi 2019, 00:44:1719.04.2019
do CCPPETMR/SIRF, Subscribed

I was just looking through the code while writing up some info on the geometrical info class, and realised:

https://github.com/CCPPETMR/SIRF/blob/master/src/common/include/sirf/common/GeometricalInfo.h#L127

The initialiser for this class should make sure that each column of the direction matrix is a vector with magnitude 1 (each axis direction is a unit vector), and that the absolute value of the determinant should be (approximately) 1 (the axes must be perpendicular).


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Richard Brown

nieprzeczytany,
23 kwi 2019, 06:07:3623.04.2019
do CCPPETMR/SIRF, Subscribed

Absolutely. These checks are done elsewhere in SIRF so this should be a simple copy and paste job.

There's still something that trips me up, however. The majority of images treated in SIRF don't need the VoxelisedGeometricalInfo. The only time we need it is when we change from one modality to another. Hence, we shouldn't throw an error if this fails.

Perhaps we print a warning if it fails and set a bool (e.g., set_up_successful) to false.

Ashley Gillman

nieprzeczytany,
23 kwi 2019, 20:41:5923.04.2019
do CCPPETMR/SIRF, Subscribed

Oh they are? Perhaps those calculations should be moved here, rather than copied.

Hmm, do you think? What if we, for example, were to decide that image addition should first verify that the two images have the same geom info? Something like that could save future difficult-to-debug issues.

It depends how images are going to be generated I guess. Wouldn't you generally read a template image, that the data would be populated from? But I would argue that regardless, those properties should always be met.

Kris Thielemans

nieprzeczytany,
24 kwi 2019, 02:28:1424.04.2019
do CCPPETMR/SIRF, Subscribed

I also vote for having checks for proper geo-info (and other things) in the construction stage. What is the use of an image without geo-info for anything to do with reconstruction or registration?

Richard Brown

nieprzeczytany,
24 kwi 2019, 05:52:1424.04.2019
do CCPPETMR/SIRF, Subscribed

I also vote for having checks for proper geo-info (and other things) in the construction stage.

Think we all agree on this.

What if we, for example, were to decide that image addition should first verify that the two images have the same geom info?

Yes, definitely. But, for example the NIfTI format can use the q- or s-form code. The s-form will get it into a standard space, whereas the q-form will get it into scanner space. What should we do if a NIfTI image only contains an s-form code? We'd have no way of comparing it to other images in scanner space (e.g., for addition).

David Atkinson

nieprzeczytany,
29 kwi 2019, 07:52:1729.04.2019
do CCPPETMR/SIRF, Subscribed

Checks at constructor stages are good. The actual code can be tricky* so it should not be repeated elsewhere (though the checks might be called again after some processing).
It can be necessary to update geom info for an existing dataset, for example after FFT, there may be a need to chop or zero pad an image, or flip/rotate to radiological presentation and this should result in a change to the geom info associated with the image.

  • I am thinking here of things like: checking unit vectors do have unit norm, that axes are orthogonal and form a right handed system, slice numbering is consistent with geometrical position.

For datasets that are simulated, I recommend the constructor does not allow a default geometry without the user explicitly asking. For example the user could pass a flag such as 'default_axial' and the constructor would set geom info for axial slices, but the key point here is that the user has to request this - otherwise there may be too much happening in the background leading to errors/misunderstandings down the line.

Odpowiedz wszystkim
Odpowiedz autorowi
Przekaż
Nowe wiadomości: 0