OSGeo / grass-addons

GRASS GIS Addons Repository
https://grass.osgeo.org/grass-stable/manuals/addons/
GNU General Public License v2.0
98 stars 150 forks source link

[Bug] i.image.bathymetry needs corrections to the R scripting calls to run all options #289

Open newcomb-d opened 3 years ago

newcomb-d commented 3 years ago

Describe the bug i.image.bathymetry can call the R GWModel program with three options, Adaptive Gaussian , Bisquare, and Fixed Gaussian. If there are issues running bisquare or adaptive, the script fails over to Fixed. With versions 3.5.3 and 3.6.3 of R, the current call to the R package raster fails and the script runs GWR in Fixed Gaussian mode , no matter what option is given in the command

To Reproduce Steps to reproduce the behavior:

  1. Load multispectral imagery into GRASS ( Sentinel 2 bands 2,3,4,8, and 11) for an area of interest with a water body for which bathymetry is needed. Also load know depth points, tide level, and a polygon area of interest.

  2. Set region to match Area of interest with resolution matching satellite imagery.

  3. Run script from the command line. ( Example) i.image.bathymetry blue_band=T18STF_20191212T155641_B02_10m green_band=T18STF_20191212T155641_B03_10m red_band=T18STF_20191212T155641_B04_10m nir_band=T18STF_20191212T155641_B08_10m band_for_correction=T18STF_20191212T155641_B11_20m calibration_points=bathy_points calibration_column=depth depth_estimate=depth_out area_of_interest=lake_Shoreline_trim_buffer

  4. See error Attaching package: 'raster'

The following object is masked from 'package:data.table':

shift

Warning message: package 'raster' was built under R version 3.5.3 Error in .read_rast_non_plugin(vname = vname, NODATA = NODATA, driverFileExt = driverFileExt, : no stars import yet Calls: readRAST -> tryCatch -> tryCatchList -> .read_rast_non_plugin Execution halted Integer outflow... Running Fixed-GWR using gaussian kernel...

Expected behavior A clear and concise description of what you expected to happen. The default should use the Adaptive Guassian kernel

Calculating optimal bandwidth using gaussian kernel.. Adaptive bandwidth: 217 CV score: 1608.003 Adaptive bandwidth: 142 CV score: 1511.212 Adaptive bandwidth: 94 CV score: 1430.953 Adaptive bandwidth: 66 CV score: 1335.32 Adaptive bandwidth: 47 CV score: 1271.241 Adaptive bandwidth: 37 CV score: 1249.903 Adaptive bandwidth: 29 CV score: 1231.113 Adaptive bandwidth: 25 CV score: 1217.598 Adaptive bandwidth: 22 CV score: 1212.767 Adaptive bandwidth: 20 CV score: 1207.13 Adaptive bandwidth: 19 CV score: 1208.674 Adaptive bandwidth: 21 CV score: 1211.075 Adaptive bandwidth: 20 CV score: 1207.13

Calculating euclidean distance

Running A-GWR using gaussian kernel Reading input data... 100% 146789 points found in input file Writing to output raster map... 100%

Screenshots If applicable, add screenshots to help explain your problem.

System description (please complete the following information):

Additional context Add any other context about the problem here.

newcomb-d commented 3 years ago

Upon further investigation, it was noted that rgeos was not loaded, Checking rgeos availability: FALSE Note: when rgeos is not available, polygon geometry computations in maptools depend on gpclib, which has a restricted licence. It is disabled by default; to enable gpclib, type gpclibPermit() and the reason that the raster module was failing was that the sp libary sp() was not being used. When line 236 if the script was modified from :
libs = ['GWmodel', 'data.table', 'rgrass7', 'rgdal', 'raster'] to: libs = ['GWmodel', 'data.table', 'sp', 'rgeos', 'rgrass7', 'rgdal', 'raster'] and the following line was added after line 248 r_file.write('use_sp()\n')

The script was then able to get past this error. However, in line 293 predict = g.read_command("g.tempfile", pid=os.getpid()).strip() + '.txt' a temporary file is written to hold the results of the GWR analysis.
On windows, this leads to a filename like this: D:\grass\lake\u18wgs84\bathy.tmp/unknown\13752.5.txt , which throws an escape character error in R on windows. This was worked around by setting a static filename in the directory that the script is run from: predict="tempgwr.txt" While this works for a desktop user and makes things os-independent, it requires the user have write access to the directory the script is running in. It might be better to set the filename with a suffix more dynamically from the os.getpid process for a result like tempgwr13725.txt in more dynamic settings.

neteler commented 3 years ago

Please consider to edit the Python script directly here and submit the changes as a pull request: https://github.com/OSGeo/grass-addons/blob/master/grass7/imagery/i.image.bathymetry/i.image.bathymetry.py

newcomb-d commented 3 years ago

Sorry for the delay. Working with external user who can submit via web interface , but is still getting up to speed on command line git , which seems to be the project preference.

Doug


From: Markus Neteler notifications@github.com Sent: Sunday, October 11, 2020 8:43 AM To: OSGeo/grass-addons grass-addons@noreply.github.com Cc: Newcomb, Doug doug_newcomb@fws.gov; Author author@noreply.github.com Subject: [EXTERNAL] Re: [OSGeo/grass-addons] [Bug] i.image.bathymetry needs corrections to the R scripting calls to run all options (#289)

This email has been received from outside of DOI - Use caution before clicking on links, opening attachments, or responding.

Please consider to edit the Python script directly here and submit the changes as a pull request: https://github.com/OSGeo/grass-addons/blob/master/grass7/imagery/i.image.bathymetry/i.image.bathymetry.py

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/OSGeo/grass-addons/issues/289#issuecomment-706699396, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABSYN7XQNMTIUKE7FRRFFVTSKGR6BANCNFSM4R3WYH2Q.

tmszi commented 3 years ago

The script was then able to get past this error. However, in line 293 predict = g.read_command("g.tempfile", pid=os.getpid()).strip() + '.txt' a temporary file is written to hold the results of the GWR analysis. On windows, this leads to a filename like this: D:\grass\lake\u18wgs84\bathy.tmp/unknown\13752.5.txt , which throws an escape character error in R on windows.

This is bug in the G__temp_element() function (line 121).

neteler commented 3 years ago

On windows, this leads to a filename like this: D:\grass\lake\u18wgs84\bathy.tmp/unknown\13752.5.txt , which throws an escape character error in R on windows.

This is bug in the G__temp_element() function (line 121).

Probably G_convert_dirseps_to_host() is missing? See https://grass.osgeo.org/programming7/paths_8c.html#a0bd23f09c109997ed8ddc25c5d650698

tmszi commented 3 years ago

On windows, this leads to a filename like this: D:\grass\lake\u18wgs84\bathy.tmp/unknown\13752.5.txt , which throws an escape character error in R on windows.

This is bug in the G__temp_element() function (line 121).

Probably G_convert_dirseps_to_host() is missing? See https://grass.osgeo.org/programming7/paths_8c.html#a0bd23f09c109997ed8ddc25c5d650698

Yes you are right or use HOST_DIRSEP.

Yesterday I prepared patch (need test on MS Win), but I have trouble with Cross-Compilation (environment) GRASS GIS for Windows OS on GNU/Linux. Otherwise I would have opened a new PR.

neteler commented 3 years ago

Right, HOST_DIRSEP is widely used in GRASS-core.

neteler commented 3 years ago

Yes you are right or use HOST_DIRSEP.

Yesterday I prepared patch (need test on MS Win), but I have trouble with Cross-Compilation (environment) GRASS GIS for Windows OS on GNU/Linux. Otherwise I would have opened a new PR.

While checking the entire core source code I found, besides lib/gis/tempfile.c two more files in which HOST_DIRSEP seems to be missing (unrelated but like worth to also fix in the same PR):

wenzeslaus commented 2 years ago

Isn't the issue more that the backslash is used rather than missing? If the error is about "an escape character" in R on Windows, that suggests that the backslashes in path are not escaped or that the string in generated R code (is there some in this case?) needs to be raw. It seems to me then that if you want to change it in grass core, you need to remove the backslashes from path rather than adding more of them and use forward slashes instead.

The question also is if g.tempfile is the right tool. Is the file huge or copied to mapset non-temporary directories? If not, standard Python temporary directory object used with a with statement and a hardcoded file name might be a better option.

newcomb-d commented 2 years ago

I just noticed a similar error for r.in.wms for 7.8.7 on Windows.