ants.transform_index_to_physical_point doesn't accept floats

[rueberger]

Calling it with a list of floats yields:

TypeError: TransformIndexToPhysicalPointF3(): incompatible function arguments. The following argument types are supported:
    1. (arg0: capsule, arg1: List[List[int]]) -> List[List[float]]

However it seems that the underlying ITK method supports floats.

This would be a highly desirable feature, costs me half a voxel of accuracy. I'm happy to take a crack at it with some guidance, as I'm not experienced with capsules.

[stnava]

the index representation does not support float as it is by definition integer. there is something incorrect about your usage if you think that you need this.

[rueberger]

Hi Brian,

As noted, the underlying implementation supports floats, there is no reason to force discretization of something that is an affine transformation under the hood. There are certainly reasons why someone would want to do such a thing, I touched on mine in #337.

[stnava]

again, I think you have a misunderstanding. the definition is integer type - not talking code here, this is the math. indices are discrete entities. the link you sent shows integer type. continuous indices are float/double - but those are just points with an identity mapping to index space. you are much better off just working with points or indices - there is no need for a continuous index.

[rueberger]

The map from physical space to index space and it's inverse are R^3 -> R^3. I do not know what you mean, it's just an affine. ants.transform_physical_point_to_index is R^3 -> R^3, there is certainly no mathematical justification for ants.transform_index_to_physical_point to be Z^3 -> R^3, and there isn't a software justification either, as there is no loss of generality for those using integers.

[stnava]

You are confusing indices and continuous indices

On Fri, Mar 18, 2022 at 9:26 AM Andrew Berger ***@***.***> wrote: The map from physical space to index space and it's inverse are R^3 -> R^3. I do not know what you mean, it's just an affine. ants.transform_physical_point_to_index is R^3 -> R^3, there is certainly no mathematical justification for ants.transform_index_to_physical_point to be Z^3 -> R^3, and there isn't a software justification either, as there is no loss of generality for those using integers. — Reply to this email directly, view it on GitHub <#343 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACPE7WSYA44WFNULW7VW5TVAR775ANCNFSM5RBLZJTQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you commented.Message ID: ***@***.***>

-- brian

[rueberger]

I don't see a useful distinction between the two. Would you accept a PR for this?

[stnava]

maybe - you would have to implement the point to continuous index transform.

indices map to a specific part of memory ( pixel / voxel ) - it has no concept of interpolation - this is important in code as it allows faster access to data. you should use indices when you want to set or get a data value in an image. otherwise, they are not useful. I almost never need to use index representation.

continuous indices are basically points without physical space.

[ntustison]

Since ITK maintains a rigid distinction between "index" and "continuous index" types (and I think that should be continued here), perhaps a separate function should be created for continuous indices.

[rueberger]

The point to continuous index transform is just a type cast. I understand the distinction I just don't think it's useful. As I said, I need the non-discretized affine. It doesn't cost anyone else anything, integers will be silently cast to floats behind the scenes.

I believe that ITK maintains a rigid distinction due to the nature of typing in C++. Note that they also have separate functions for the inverse physical to index transform, for which ants returns a float array and is the more relevant direction in regards to the discussion of the usefulness of the distinction between float and int indices (I still don't think it's useful though). In that direction (to index), you have to explicitly cast to int. If I'm coming from an index I have already done my indexing and it doesn't matter whether things are getting silently cast to floats behind the scenes.

Another perspective is that the silent cast to float is already happening somewhere in the pipeline for the affine matmul. Now or later, the cast has to happen.

[ntustison]

So what's wrong with having a separate function?

[stnava]

indices map to a specific part of memory ( pixel / voxel ) - it has no concept of interpolation

index access is fast + cheap b/c there is no affine tx to worry about. it's closer to direct memory access ... which is the fastest there is. that is the reason for the distinction.

[stnava]

the reason I know this is b/c I was there in the early 2000s when the original itk implementation discussions were had.

[rueberger]

This has nothing to do with indexing, it's just the inverse of the affine. This method isn't used anywhere in the codebase and isn't even documented.

Having a separate method would be redundant, confusing, and unpythonic. It's an implementation detail. We should take advantage of python being dynamically typed to make the library cleaner.

[stnava]

If this has nothing to do with indexing then why are you using that method at all? I am completely confused about what you want to do here now

On Fri, Mar 18, 2022 at 11:42 AM Andrew Berger ***@***.***> wrote: This has nothing to do with indexing, it's just the inverse of the affine. This method isn't used anywhere in the codebase and isn't even documented. Having a separate method would be redundant, confusing, and unpythonic. It's an implementation detail. We should take advantage of python being dynamically typed to make the library cleaner. — Reply to this email directly, view it on GitHub <#343 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACPE7W3R6B526KSU42JBE3VASP5VANCNFSM5RBLZJTQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you commented.Message ID: ***@***.***>

-- brian

[rueberger]

As discussed in #337, I have a pipeline involving a third party displacement field defined in index space. The coordinates I am transforming have a measurement error considerably smaller than a voxel and I would like to retain that subvoxel precision.

[stnava]

OK. But then you are really outside of what we are trying to support here within the core package. The definition of point to index transformations and things like that are not up for debate. They’ve been this way for over 20 years. And for good reasons. It may just be worth developing a small python package that has the necessary translations for your purposes.

On Fri, Mar 18, 2022 at 11:50 AM Andrew Berger ***@***.***> wrote: As discussed in #337 <#337>, I have a pipeline involving a third party displacement field defined in index space. The coordinates I am transforming have a measurement error considerably smaller than a voxel and I would like to retain that subvoxel precision. — Reply to this email directly, view it on GitHub <#343 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACPE7WTNQWF63KGGET26ILVASQ3TANCNFSM5RBLZJTQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you commented.Message ID: ***@***.***>

-- brian

[rueberger]

I would just do it on my fork as it is a quite minor change. I think we must be miscommunicating as this change would have no consequence to the end user or architecture of the library. I do not see a downside. One with an integer index could simple carry on using the method as they were before with no changes in behavior.