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

R version issues #389

Closed muratmaga closed 3 months ago

muratmaga commented 5 months ago

I recently moved to R 4.3.1 from 4.1.2. This line of code for two step registrations seems to work fine in 4.1.2, with a warning about transform orders (which I always got).

 fi <- antsImageRead(getANTsRData("r16") )
 mi <- antsImageRead(getANTsRData("r64") )
 mytx2 <- antsRegistration(fixed=fi,  moving=mi, typeofTransform = 'SyN')
 mytx3 <- antsRegistration(fixed=fi,  moving=mi, typeofTransform = 'SyNCC', initialTransform=mytx2$fwdtransforms)

but same code fails with this error in 4.3.1 Error in if (is.na(initx)) { : the condition has length > 1

Here is the session info for both R versions. As far as I can tell both have the same ANTs versions. Not sure where the error is coming from.

R4.1.2

> antsVersions
  Dependency                                   GitTag
1       ANTs d30526f9cb5159bc0a3e9011f7ae5f409b3634c8
2  ANTsRCore 8e234fd1363c0d618f9dc21c9566f3d5464655a2
3    ANTsURL        https://github.com/ANTsX/ANTs.git
4        ITK                                 v5.3rc04

sessionInfo()
R version 4.1.2 (2021-11-01)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04.6 LTS

Matrix products: default
BLAS:   /usr/lib/x86_64-linux-gnu/blas/libblas.so.3.9.0
LAPACK: /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.9.0

locale:
 [1] LC_CTYPE=en_CA.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=en_CA.UTF-8        LC_COLLATE=en_CA.UTF-8    
 [5] LC_MONETARY=en_CA.UTF-8    LC_MESSAGES=en_CA.UTF-8   
 [7] LC_PAPER=en_CA.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_CA.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] ANTsR_0.5.7.5   ANTsRCore_0.7.5

loaded via a namespace (and not attached):
[1] compiler_4.1.2      RcppEigen_0.3.3.9.3 magrittr_2.0.3     
[4] Matrix_1.4-0        tools_4.1.2         ITKR_0.6.0.0.2     
[7] Rcpp_1.0.9          grid_4.1.2          lattice_0.20-45    

R 4.3.1:

  Dependency                                   GitTag
1       ANTs d30526f9cb5159bc0a3e9011f7ae5f409b3634c8
2  ANTsRCore 8e234fd1363c0d618f9dc21c9566f3d5464655a2
3    ANTsURL        https://github.com/ANTsX/ANTs.git
4        ITK                                 v5.3rc04

R version 4.3.1 (2023-06-16)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04.6 LTS

Matrix products: default
BLAS:   /usr/lib/x86_64-linux-gnu/blas/libblas.so.3.9.0 
LAPACK: /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.9.0

locale:
 [1] LC_CTYPE=en_CA.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=en_CA.UTF-8        LC_COLLATE=en_CA.UTF-8    
 [5] LC_MONETARY=en_CA.UTF-8    LC_MESSAGES=en_CA.UTF-8   
 [7] LC_PAPER=en_CA.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_CA.UTF-8 LC_IDENTIFICATION=C       

time zone: America/Vancouver
tzcode source: system (glibc)

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] ANTsR_0.5.7.5   ANTsRCore_0.7.5

loaded via a namespace (and not attached):
[1] compiler_4.3.1      RcppEigen_0.3.3.9.3 magrittr_2.0.3     
[4] Matrix_1.4-0        tools_4.3.1         ITKR_0.6.0.0.2     
[7] Rcpp_1.0.9          grid_4.3.1          lattice_0.20-45  
jefferis commented 5 months ago

I think the problem (for R >=4.2) is here over in ANTsRCore:

https://github.com/ANTsX/ANTsRCore/blob/1c79a80c3e03216c2456b0f81d8628dbc91859d7/R/antsRegistration.R#L393-L395

The breaking change in R 4.2.0 is documented in R news as follows:

Calling if() or while() with a condition of length greater than one gives an error rather than a warning. Consequently, environment variable _R_CHECK_LENGTH_1CONDITION no longer has any effect.

I guess you can do something like

if (all(is.na(initx))) { 
   initx = paste("[", f, ",", m, ",1]", sep = "") 
 } 

to ensure that the result is a logical of length 1 or check for whatever else distinguishes between the signalling value of NA vs when initx contains a transform.

muratmaga commented 5 months ago

Fix works fine, thank you.

I wonder if this needs to be committed to the repo? Because this is going to fail for anyone who has more than items in the transform. if this is a frowned upon thing (i.e., the goal of transformList is only to pass a single transform, like a prior affine), than perhaps documentation needs to be updated.

cookpa commented 3 months ago

In the current implementation, multiple transform files should be allowed, but multiple linear antsTransform objects are not. There's some manipulation of the antsTransform objects that I don't understand, so I've left it alone. But it should support using a vector of file names like my_registration$fwdtransforms. Fixed and documented in #397