Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FATAL ERROR: DgQ2DDtoIConverter::convertTypedAddress() #63

Closed
LewisAJones opened this issue Feb 7, 2023 · 7 comments
Closed

FATAL ERROR: DgQ2DDtoIConverter::convertTypedAddress() #63

LewisAJones opened this issue Feb 7, 2023 · 7 comments

Comments

@LewisAJones
Copy link

Hi @r-barnes,

First, thanks for getting dggridR back on CRAN - I'm really glad to see it back!

I've recently been rerunning some of my R scripts and I am getting some fatal error messages (even when using your README example).

library(dggridR)
library(dplyr)

#Construct a global grid with cells approximately 1000 miles across
dggs          <- dgconstruct(spacing=1000, metric=FALSE, resround='down')

#Load included test data set
data(dgquakes)

#Get the corresponding grid cells for each earthquake epicenter (lat-long pair)
dgquakes$cell <- dgGEO_to_SEQNUM(dggs, dgquakes$lon, dgquakes$lat)$seqnum

Returns:

FATAL ERROR: DgQ2DDtoIConverter::convertTypedAddress():  coordinate out of range: (0, 9)
FATAL ERROR: DgQ2DDtoIConverter::convertTypedAddress():  coordinate out of range: (6, 9)
FATAL ERROR: DgQ2DDtoIConverter::convertTypedAddress():  coordinate out of range: (9, 3)
FATAL ERROR: DgQ2DDtoIConverter::convertTypedAddress():  coordinate out of range: (0, 9)
...

... And so on

> sessionInfo()
R version 4.2.2 (2022-10-31)
Platform: aarch64-apple-darwin20 (64-bit)
Running under: macOS Ventura 13.0.1

Matrix products: default
LAPACK: /Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

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

other attached packages:
[1] dggridR_3.0.0 sp_1.6-0      sf_1.0-9      rlang_1.0.6   dplyr_1.1.0  

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.10        rstudioapi_0.14    magrittr_2.0.3     units_0.8-1        tidyselect_1.2.0   lattice_0.20-45   
 [7] R6_2.5.1           fansi_1.0.4        tools_4.2.2        grid_4.2.2         KernSmooth_2.23-20 utf8_1.2.3        
[13] cli_3.6.0          e1071_1.7-13       DBI_1.1.3          class_7.3-20       tibble_3.1.8       lifecycle_1.0.3   
[19] vctrs_0.5.2        glue_1.6.2         proxy_0.4-27       compiler_4.2.2     pillar_1.8.1       generics_0.1.3    
[25] classInt_0.4-8     pkgconfig_2.0.3 

This error seems to be independent of spacing size.

Any ideas on a fix for this or what is going on? And apologies if I've missed something obvious!

Thanks,

Lewis

@jsocolar
Copy link

jsocolar commented Mar 5, 2023

I'm seeing the same, but only on one of my machines (an M1 mac; I also have an intel mac that runs everything just fine). I don't know if the behavior is specific to M1 or to some other difference between the machines (compiler toolchain, OS, goodness knows what else), but I see that @LewisAJones was on M1 and also that a related issue here #54 suggests that the M1 architecture is implicated. In this comment I'll use "M1" and "Intel" as shorthands for everything that might be different between the machines.

The behavior is pretty spooky, and comes from dggridR:::GEO_to_SEQNUM. On the M1, if I do:

library(dggridR)
grid_large <- dgconstruct(res = 6)
dgGEO_to_SEQNUM(grid_large, -85, 30)

I get

FATAL ERROR: DgQ2DDtoIConverter::convertTypedAddress():  coordinate out of range: (27, 25)
$seqnum
[1] 729

This is particularly dangerous, because the fatal error didn't actually raise an error in the R session, it did produce a syntactically correct output, and the output is wrong. On an intel mac, I get 3672 as the seqnum, with no FATAL ERROR message.

But it gets spookier, and even (much) more dangerous.

The coordinates where the M1 raises the message and then fails are geographically coherent. For example:

# These all raise the `FATAL ERROR` message and return 729
dgGEO_to_SEQNUM(grid_large, -84.20946, 30.3995)
dgGEO_to_SEQNUM(grid_large, -84.2, 30.4)
dgGEO_to_SEQNUM(grid_large, -85, 30)
dgGEO_to_SEQNUM(grid_large, -85, 31)
dgGEO_to_SEQNUM(grid_large, -84, 31)
dgGEO_to_SEQNUM(grid_large, -83, 31)

# If we move around enough, we (seemingly) flip over to an adjacent cell. Still raises
# `FATAL ERROR`, but now with seqnum = 730
dgGEO_to_SEQNUM(grid_large, -82, 31)

# These do not raise the `FATAL ERROR` message, and they return either 3671, 3672, or 3698
dgGEO_to_SEQNUM(grid_large, -85, 32)
dgGEO_to_SEQNUM(grid_large, -84, 30)
dgGEO_to_SEQNUM(grid_large, -85, 33)
dgGEO_to_SEQNUM(grid_large, -85, 29)
dgGEO_to_SEQNUM(grid_large, -85, 28)

HOWEVER, although on the M1 mac dgGEO_to_SEQNUM(grid_large, -85, 32) does not raise the FATAL ERROR message and returns $seqnum = 3671 (see above), the Intel mac returns a DIFFERENT seqnum $seqnum = 728 (and still no errors)!!! Thus, an M1 user could get the wrong behavior silently!

I've confirmed that this behavior comes directly from the dggridR:::GEO_to_SEQNUM call and doesn't depend on any usual suspects such as the state of sf_use_s2().

So then I reverted to d92fc92, and all of the weird behavior on the M1 vanished. No more FATAL ERROR messages; all seqnums mentioned above consistent with the Intel mac running the CRAN version. More spelunking and waiting to compile the C++... and we have our culprit in 6d9129a. There, the behavior is similar to the contemporary CRAN behavior, except that rather than seeing the FATAL ERROR messages, the "bad" coordinates just crash my R session. If I run any of the "good" coordinates (including the one that gives the apparently wrong seqnum), I get the same (sometimes incorrect) seqnum as I did running the CRAN version on the M1.

I don't have any brilliant ideas about what to do here, except to mention to @LewisAJones that you might be able to make progress by reverting with remotes::install_github("r-barnes/dggridR", ref = "ec2a040") (the commit before 6d9129a). But the bigger problem is the behavior on dgGEO_to_SEQNUM(grid_large, -85, 30), because the user won't even see the weird FATAL ERROR message to follow up on and hopefully land here.

Here's the sessionInfo from the M1, which is where all the problems are occurring:

> sessionInfo()
R version 4.2.2 (2022-10-31)
Platform: aarch64-apple-darwin20 (64-bit)
Running under: macOS Monterey 12.2

Matrix products: default
LAPACK: /Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

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

other attached packages:
[1] dggridR_3.0.0 sp_1.6-0      sf_1.0-9      rlang_1.0.6   dplyr_1.1.0  

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.10        rstudioapi_0.14    magrittr_2.0.3     units_0.8-1       
 [5] tidyselect_1.2.0   lattice_0.20-45    R6_2.5.1           fansi_1.0.4       
 [9] tools_4.2.2        grid_4.2.2         KernSmooth_2.23-20 utf8_1.2.3        
[13] cli_3.6.0          e1071_1.7-13       DBI_1.1.3          class_7.3-21      
[17] tibble_3.1.8       lifecycle_1.0.3    vctrs_0.5.2        glue_1.6.2        
[21] proxy_0.4-27       compiler_4.2.2     pillar_1.8.1       generics_0.1.3    
[25] classInt_0.4-9     boot_1.3-28.1      pkgconfig_2.0.3  

SebKrantz added a commit to SebKrantz/dggridR that referenced this issue Jul 15, 2023
… of a double (factor) to long int here: on M1 Macs this is rounded downwards, thus I needed to add a small number. Took me 6 freaking hours of simultaneous debugging on Mac and Windows to find it...
@SebKrantz
Copy link
Collaborator

This issue has been fixed in PR #66, at least for the use cases flagged above. While this PR is not merged, Apple Silicon users can install from my fork e.g. remotes::install_github("SebKrantz/dggridR").

@Daniel-Huff
Copy link

@SebKrantz i updated my dggridR 3.0.0 to 3.1.0 follow the code
library(remotes) remotes::install_github("SebKrantz/dggridR")
but it still not works well when i run
`library(dggridR)

#Generate a dggs specifying an intercell spacing of ~25 miles
dggs <- dgconstruct(spacing=100, metric=FALSE, resround='nearest')

#Read in the South Africa's borders from the shapefile
sa_border <- st_read(dg_shpfname_south_africa(), layer="ZAF_adm0")
st_crs(sa_border) = 4326

#Get a grid covering South Africa
sa_grid <- dgshptogrid(dggs, dg_shpfname_south_africa())

#Plot South Africa's borders and the associated grid
p <- ggplot() +
geom_sf(data=sa_border, fill=NA, color="black") +
geom_sf(data=sa_grid, fill=alpha("blue", 0.4), color=alpha("white", 0.4))
p
`

got figure
image

@SebKrantz
Copy link
Collaborator

SebKrantz commented Aug 6, 2023 via email

@Daniel-Huff
Copy link

Thanks for your response.
after adding library(sp) library(sf)to my script.it works well for me on Apple M1 .
i think maybe dggridR 3.1.0 solve the hole

@SebKrantz
Copy link
Collaborator

SebKrantz commented Aug 7, 2023

@Daniel-Huff, sp was removed in 3.1.0 thus you don't need to import it. I am also unable to reproduce your issue using a M1 mac:

library(dggridR)
library(ggplot2)
library(sf)
#> Linking to GEOS 3.11.0, GDAL 3.5.3, PROJ 9.1.0; sf_use_s2() is TRUE

#Generate a dggs specifying an intercell spacing of ~25 miles
dggs <- dgconstruct(spacing=100, metric=FALSE, resround='nearest')
#> Resolution: 7, Area (mi^2): 14491.99750509, Spacing (mi): 93.7218051701576, CLS (mi): 107.077441232619

#Read in the South Africa's borders from the shapefile
sa_border <- st_read(dg_shpfname_south_africa(), layer="ZAF_adm0")
#> Reading layer `ZAF_adm0' from data source 
#>   `/Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/library/dggridR/extdata/ZAF_adm0.shp' 
#>   using driver `ESRI Shapefile'
#> Simple feature collection with 1 feature and 1 field
#> Geometry type: MULTIPOLYGON
#> Dimension:     XY
#> Bounding box:  xmin: 16.45802 ymin: -46.93639 xmax: 37.87945 ymax: -22.14108
#> CRS:           NA
st_crs(sa_border) = 4326

#Get a grid covering South Africa
sa_grid <- dgshptogrid(dggs, sa_border)

#Plot South Africa's borders and the associated grid
p <- ggplot() +
  geom_sf(data=sa_border, fill=NA, color="black") +
  geom_sf(data=sa_grid, fill=alpha("blue", 0.4), color=alpha("white", 0.4))
p

Created on 2023-08-07 with reprex v2.0.2

Note that in v3.1.0, you can also direcly pass the sf data frame to dgshptogrid().
Given that this is working well and solving people's problems, it would be nice if @r-barnes could still consider merging it and pushing it to CRAN.

r-barnes pushed a commit that referenced this issue May 21, 2024
…ome optimization

* Small fix.
* Need to add sf and rename count column.
* Using R's numeric() instead of alloc().
* Update NEWS
* Fixing #63. The issue on M1 Mac is due to implicit conversion of a double (factor) to long int here: on M1 Macs this is rounded downwards, thus I needed to add a small number. Took me 6 freaking hours of simultaneous debugging on Mac and Windows to find it...
* Adding return_sf argument: set to FALSE for memory efficient long datat frame (can be converted to S2 and other classes).
* Appeasing R CMD check.
* Comment and change of notation as requested by @r-barnes.
* Using dgprintf as requested by @r-barnes.
* Update CONTRIBUTING.md to explain C++ workflow
* Also using dgprintf here.
* Install instructions for fork.
* Update README.md
* Remove magrittr pipe.
* Need to add dplyr for vignette it seems.
r-barnes pushed a commit that referenced this issue May 21, 2024
…ome optimization

* Small fix.
* Need to add sf and rename count column.
* Using R's numeric() instead of alloc().
* Update NEWS
* Fixing #63. The issue on M1 Mac is due to implicit conversion of a double (factor) to long int here: on M1 Macs this is rounded downwards, thus I needed to add a small number. Took me 6 freaking hours of simultaneous debugging on Mac and Windows to find it...
* Adding return_sf argument: set to FALSE for memory efficient long datat frame (can be converted to S2 and other classes).
* Appeasing R CMD check.
* Comment and change of notation as requested by @r-barnes.
* Using dgprintf as requested by @r-barnes.
* Update CONTRIBUTING.md to explain C++ workflow
* Also using dgprintf here.
* Install instructions for fork.
* Update README.md
* Remove magrittr pipe.
* Need to add dplyr for vignette it seems.
r-barnes pushed a commit that referenced this issue May 21, 2024
…ome optimization

* Small fix.
* Need to add sf and rename count column.
* Using R's numeric() instead of alloc().
* Update NEWS
* Fixing #63. The issue on M1 Mac is due to implicit conversion of a double (factor) to long int here: on M1 Macs this is rounded downwards, thus I needed to add a small number. Took me 6 freaking hours of simultaneous debugging on Mac and Windows to find it...
* Adding return_sf argument: set to FALSE for memory efficient long datat frame (can be converted to S2 and other classes).
* Appeasing R CMD check.
* Comment and change of notation as requested by @r-barnes.
* Using dgprintf as requested by @r-barnes.
* Update CONTRIBUTING.md to explain C++ workflow
* Also using dgprintf here.
* Install instructions for fork.
* Update README.md
* Remove magrittr pipe.
* Need to add dplyr for vignette it seems.
@SebKrantz
Copy link
Collaborator

This was due to a numeric precision error in the C++ source code visible on systems without a long double type - such as Apple Silicon. It should be fixed by PR #66, at least in my experience as a M1 Mac user.

SebKrantz added a commit that referenced this issue Jul 24, 2024
* Getting rid of dplyr and sp: replacing with collapse, sf and s2 + some optimization

* Small fix.

* Need to add sf and rename count column.

* Using R's numeric() instead of alloc().

* Update NEWS

* Fixing #63. The issue on M1 Mac is due to implicit conversion of a double (factor) to long int here: on M1 Macs this is rounded downwards, thus I needed to add a small number. Took me 6 freaking hours of simultaneous debugging on Mac and Windows to find it...

* Adding return_sf argument: set to FALSE for memory efficient long datat frame (can be converted to S2 and other classes).

* Appeasing R CMD check.

* Comment and change of notation as requested by @r-barnes.

* Update CONTRIBUTING.md to explain C++ workflow

* Using dgprintf as requested by @r-barnes.

* Also using dgprintf here.

* Install instructions for fork.

* Update README.md

* Remove magrittr pipe.

* Need to add dplyr for vignette it seems.

* Fix C++ issues.

* Change of maintainer.

* Spacing.

* Remove duplicate.

* Remove installation notes.

* Fix spacing.

---------

Co-authored-by: Richard Barnes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants