search

ANTsX / ANTsR

R interface to the ANTs biomedical image processing library
https://antsx.github.io/ANTsR
Apache License 2.0
127 stars 35 forks source link

Subtraction methods fail in aslAveraging #319

Open s-kash opened 4 years ago

s-kash commented 4 years ago

Hi ANTsR team,

Describe the bug I'm trying the ASL processing tools available in ANTsR. Particularly to obtain pair-wise differences of a timeseries. I want to compare the different perfusion subtraction methods available. However, all four: "simpleSubtract","surroundSubtract","cubicSubtract" and ""sincSubtract" all crash with the following error message (example for one method):

ITK ExceptionObject caught! Error in aslAveraging(input_asl, mask = NULL, method = "surroundSubtract") : /usr/local/lib/R/site-library/ITKR/libs/include/ITK-5.1/itkImageSource.hxx:268: itk::ERROR: AverageOverDimensionImageFilter(0x563b5e72cf10): Subclass should override this method!!! If old behavior is desired invoke this->DynamicMultiThreadingOff(); before Update() is called. The best place is in class constructor.

To Reproduce Steps to reproduce the behavior: input_asl <- antsImageRead("test_asl_timeseries.nii.gz") perf_asl_surrsub <- aslAveraging(input_asl, mask = NULL, method = "surroundSubtract")

Expected behavior I expect the command to provide the temporal mean of the pair-wise difference of the ASL time-series. Isn't this right? Although, I'd prefer it give me the entire pair-wise difference time-series (before averaging) as well.

Desktop (please complete the following information):

Additional context Rstudio v 1.2.5001 and R version 3.6.3

Help is appreciated.

Thanks, Sri

stnava commented 4 years ago

@jeffduda any chance. you could take a look at this?

jeffduda commented 4 years ago

This appears to be an ANTs problem that is manifesting in ANTsR. It should just be a one line fix. I can fix it and push it if you want, or would you prefer a PR? Obviously, this fix won't filter up to ANTsR until the ANTs dependency is updated to a version with the fix. Not sure what the current policy is regarding how often the version of ANTs used by ANTsR is updated.

On Thu, Jul 16, 2020 at 7:50 AM stnava notifications@github.com wrote:

@jeffduda https://github.com/jeffduda any chance. you could take a look at this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659359993, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5NCFD6TUTQMW4RMPUQMTR33SPDANCNFSM4NMPEKJA .

stnava commented 4 years ago

No need for PR.

We are happy to do updates to tags as regularly as needed to fix bugs.

On Thu, Jul 16, 2020 at 5:47 PM Jeffrey Duda notifications@github.com wrote:

This appears to be an ANTs problem that is manifesting in ANTsR. It should just be a one line fix. I can fix it and push it if you want, or would you prefer a PR? Obviously, this fix won't filter up to ANTsR until the ANTs dependency is updated to a version with the fix. Not sure what the current policy is regarding how often the version of ANTs used by ANTsR is updated.

On Thu, Jul 16, 2020 at 7:50 AM stnava notifications@github.com wrote:

@jeffduda https://github.com/jeffduda any chance. you could take a look at this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659359993, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAD5NCFD6TUTQMW4RMPUQMTR33SPDANCNFSM4NMPEKJA

.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659692254, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACPE7X5ZSUDFURN6WY3XU3R35YNPANCNFSM4NMPEKJA .

--

brian

jeffduda commented 4 years ago

Ok, I pushed the fix to ANTs and all looks good. I don't have push permission for ANTsRCore, but all you need to do is update the configure file with the new ANTs commit tag. I'll put up a PR with that tomorrow if you haven't had a chance to get to that. No changes to ANTsR itself, but it will need to be re-compiled. -jeff

On Thu, Jul 16, 2020 at 5:59 PM stnava notifications@github.com wrote:

No need for PR.

We are happy to do updates to tags as regularly as needed to fix bugs.

On Thu, Jul 16, 2020 at 5:47 PM Jeffrey Duda notifications@github.com wrote:

This appears to be an ANTs problem that is manifesting in ANTsR. It should just be a one line fix. I can fix it and push it if you want, or would you prefer a PR? Obviously, this fix won't filter up to ANTsR until the ANTs dependency is updated to a version with the fix. Not sure what the current policy is regarding how often the version of ANTs used by ANTsR is updated.

On Thu, Jul 16, 2020 at 7:50 AM stnava notifications@github.com wrote:

@jeffduda https://github.com/jeffduda any chance. you could take a look at this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659359993, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AAD5NCFD6TUTQMW4RMPUQMTR33SPDANCNFSM4NMPEKJA

.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659692254, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AACPE7X5ZSUDFURN6WY3XU3R35YNPANCNFSM4NMPEKJA

.

--

brian

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659697642, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5NCFCYRHKPEPKHBPZUETR35Z3NANCNFSM4NMPEKJA .

stnava commented 4 years ago

many thanks - yes a PR to antsrcore would be helpful.

brian

On Thu, Jul 16, 2020 at 7:35 PM Jeffrey Duda notifications@github.com wrote:

Ok, I pushed the fix to ANTs and all looks good. I don't have push permission for ANTsRCore, but all you need to do is update the configure file with the new ANTs commit tag. I'll put up a PR with that tomorrow if you haven't had a chance to get to that. No changes to ANTsR itself, but it will need to be re-compiled. -jeff

On Thu, Jul 16, 2020 at 5:59 PM stnava notifications@github.com wrote:

No need for PR.

We are happy to do updates to tags as regularly as needed to fix bugs.

On Thu, Jul 16, 2020 at 5:47 PM Jeffrey Duda notifications@github.com wrote:

This appears to be an ANTs problem that is manifesting in ANTsR. It should just be a one line fix. I can fix it and push it if you want, or would you prefer a PR? Obviously, this fix won't filter up to ANTsR until the ANTs dependency is updated to a version with the fix. Not sure what the current policy is regarding how often the version of ANTs used by ANTsR is updated.

On Thu, Jul 16, 2020 at 7:50 AM stnava notifications@github.com wrote:

@jeffduda https://github.com/jeffduda any chance. you could take a look at this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659359993, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AAD5NCFD6TUTQMW4RMPUQMTR33SPDANCNFSM4NMPEKJA

.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659692254, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AACPE7X5ZSUDFURN6WY3XU3R35YNPANCNFSM4NMPEKJA

.

--

brian

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659697642, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAD5NCFCYRHKPEPKHBPZUETR35Z3NANCNFSM4NMPEKJA

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659734816, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACPE7QUHPG4KPIT2ZDFXN3R36FEHANCNFSM4NMPEKJA .

jeffduda commented 4 years ago

PR submitted

On Thu, Jul 16, 2020 at 7:40 PM stnava notifications@github.com wrote:

many thanks - yes a PR to antsrcore would be helpful.

brian

On Thu, Jul 16, 2020 at 7:35 PM Jeffrey Duda notifications@github.com wrote:

Ok, I pushed the fix to ANTs and all looks good. I don't have push permission for ANTsRCore, but all you need to do is update the configure file with the new ANTs commit tag. I'll put up a PR with that tomorrow if you haven't had a chance to get to that. No changes to ANTsR itself, but it will need to be re-compiled. -jeff

On Thu, Jul 16, 2020 at 5:59 PM stnava notifications@github.com wrote:

No need for PR.

We are happy to do updates to tags as regularly as needed to fix bugs.

On Thu, Jul 16, 2020 at 5:47 PM Jeffrey Duda <notifications@github.com

wrote:

This appears to be an ANTs problem that is manifesting in ANTsR. It should just be a one line fix. I can fix it and push it if you want, or would you prefer a PR? Obviously, this fix won't filter up to ANTsR until the ANTs dependency is updated to a version with the fix. Not sure what the current policy is regarding how often the version of ANTs used by ANTsR is updated.

On Thu, Jul 16, 2020 at 7:50 AM stnava notifications@github.com wrote:

@jeffduda https://github.com/jeffduda any chance. you could take a look at this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659359993 , or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AAD5NCFD6TUTQMW4RMPUQMTR33SPDANCNFSM4NMPEKJA

.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659692254, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AACPE7X5ZSUDFURN6WY3XU3R35YNPANCNFSM4NMPEKJA

.

--

brian

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659697642, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AAD5NCFCYRHKPEPKHBPZUETR35Z3NANCNFSM4NMPEKJA

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659734816, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AACPE7QUHPG4KPIT2ZDFXN3R36FEHANCNFSM4NMPEKJA

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659736379, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5NCGKYYJ5TOZESXAJELTR36FV3ANCNFSM4NMPEKJA .

jeffduda commented 4 years ago

it would be pretty easy to implement the request of returning difference pairs (for relevant methods). That would be an ANTsR update. Should I add that in (in a backwards compatible way) ? -jeff

On Sun, Jul 19, 2020 at 4:09 PM Jeffrey Duda jeff.duda@gmail.com wrote:

PR submitted

On Thu, Jul 16, 2020 at 7:40 PM stnava notifications@github.com wrote:

many thanks - yes a PR to antsrcore would be helpful.

brian

On Thu, Jul 16, 2020 at 7:35 PM Jeffrey Duda notifications@github.com wrote:

Ok, I pushed the fix to ANTs and all looks good. I don't have push permission for ANTsRCore, but all you need to do is update the configure file with the new ANTs commit tag. I'll put up a PR with that tomorrow if you haven't had a chance to get to that. No changes to ANTsR itself, but it will need to be re-compiled. -jeff

On Thu, Jul 16, 2020 at 5:59 PM stnava notifications@github.com wrote:

No need for PR.

We are happy to do updates to tags as regularly as needed to fix bugs.

On Thu, Jul 16, 2020 at 5:47 PM Jeffrey Duda < notifications@github.com> wrote:

This appears to be an ANTs problem that is manifesting in ANTsR. It should just be a one line fix. I can fix it and push it if you want, or would you prefer a PR? Obviously, this fix won't filter up to ANTsR until the ANTs dependency is updated to a version with the fix. Not sure what the current policy is regarding how often the version of ANTs used by ANTsR is updated.

On Thu, Jul 16, 2020 at 7:50 AM stnava notifications@github.com wrote:

@jeffduda https://github.com/jeffduda any chance. you could take a look at this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659359993 , or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AAD5NCFD6TUTQMW4RMPUQMTR33SPDANCNFSM4NMPEKJA

.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659692254, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AACPE7X5ZSUDFURN6WY3XU3R35YNPANCNFSM4NMPEKJA

.

--

brian

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659697642, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AAD5NCFCYRHKPEPKHBPZUETR35Z3NANCNFSM4NMPEKJA

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659734816, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AACPE7QUHPG4KPIT2ZDFXN3R36FEHANCNFSM4NMPEKJA

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/319#issuecomment-659736379, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5NCGKYYJ5TOZESXAJELTR36FV3ANCNFSM4NMPEKJA .

stnava commented 4 years ago

that sounds good - I just tested the surround subtract and it worked in the sense that it did not crash. will try some time later this month on real data.