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

# GRASS write permissions in Windows hosted docker environments #702

Closed
wants to merge 2 commits into from

Conversation

JimColl
Copy link

@JimColl JimColl commented Oct 6, 2022

Issue:

Somewhere between the FIM3 and FIM4 transition and my own transition to distrod following the de-FOSS-ing of docker desktop and transition away from virtualbox, the GRASS environment broke and will no longer successfully execute. Branch 0 fails to process with mapset permission errors, the root of which stem from the r_grow_distance function.
image
The r_grow_distance function is used to generate "proximity" (Euclidean distance) and allocation (Euclidean allocation) rasters using the grass environment. Attempts to circumvent permissions (windows side explicit permission settings, reinstallation, writing to container internal folder) did not work. I was unable to identify a specific cause for this error, but this SE was about as relevant an issue as I could find but did not provide a workaround. Since the use of GRASS is limited to only a few locations, I transitioned all occurrences of r_grow_distance to their WhiteboxTools equivalents.

Changes to files and outputs

  • Docker image updates

Walked thought pipenv install procedure with Rob. For posterity, from within docker:

cd /
pipenv uninstall grass-sessions
pipenv install whitebox
cp /Pipfile /foss_fim/Pipfile
cp /Pipfile.lock /foss_fim/Pipfile.lock
  • src/agreedem.py and src/unique_pixel_and_allocation.py both received overhauls to run whiteboxtools in place of grass. r.grow.distance abstracts some preprocessing needed to run whitebox, so extra intermediate binary (presence or absence) rasters are created and cleaned as needed. These include:
    • _allo.tif and _dist.tif, both of which are explicitly removed with the -t flag of the __ call.
  • r_grow_distance.py was removed.
  • The grass environment flag was dropped from src/gms/delineate_hydros_and_produce_HAND.sh and src/run_by_unit.sh

Testing

Tested using hucs '08040101 11010009 11090204 11110202', subtracting the rem generated using this image on my own hardware with the equivalent rem from OWP. You can pull mine in egress-free from https://waterduck.ddns.net:9000/wbt_validation/{huc8}/branches/0/…

  • Timing
    • Note: Timing seems to not have been affected or slightly improved (72 minutes vs 122) but is not a 1:1 comparison
  • Parity
    • Built and deployed on dev1
    • Based on methodology above, rems 0 out. Here blue is 0, red is no_data
    • More images inbound.

rem_comp

  • vis team conformant
    • Is there a process I can run to determine if this works in hydrovis workflows?

Edge cases and errors introduced

  1. Outside OWP installation headaches
  2. To run Euclidean distance, whitebox expects a binary raster whereas grass did not. To accomplish this I altered src/unique_pixel_and_allocation.py to return 0's instead of no_data. In the instance that the uppermost corner of a processing unit happens to be a stream, I believe that that pixel is masked from processing.

@JamesColl-NOAA JamesColl-NOAA self-assigned this Oct 6, 2022
@JamesColl-NOAA JamesColl-NOAA added testing Evaluation and testing related refactoring Refacting code to obtain the same result dependencies Pull requests that update a dependency file FIM4 FIM3 and removed FIM3 labels Oct 6, 2022
@RobHanna-NOAA
Copy link
Contributor

Based on what we have as of Oct 10, I did a docker build, gms_pipeline and alpha test (synthesize_test_cases.py) and it all looked good. I stumble across one new update that is required.

Some files are not being cleanup correctly in the branch 0 folder only (rest are fine).
Files requiring cleanup are:

  • agree_binary_bufgrid.tif
  • agree_bufgrid_zerod.tif
  • agree_smogrid_zerod.tif

For the files deny_gms_branches_prod.lst, deny_gms_branches_dev.lst, deny_gms_branch_unittests.lst, and deny_gms_branch_zero.lst, those three files need to be added to the list in order to removed.

However... if the three files are needed until all branches are fully processed, then simply add a # to the start of all three files in the deny_gms_branch_zero.lst which will mean they will NOT be removed during initial branch zero cleanup.

@mluck
Copy link
Contributor

mluck commented Oct 11, 2022

I'm seeing a difference in the REM grids between the FIM4 (dev) version based on GRASS and this whitebox version. It seems that the AGREE distance and allocation grids are somewhat different between the two given the same input data.

@JimColl JimColl changed the base branch from dev to dev_fim4_wbt October 12, 2022 19:52
@mluck
Copy link
Contributor

mluck commented Jan 17, 2023

The difference between dem_burned_filled.tif from GRASS and Whitebox shows a checkerboard pattern (max differences of +/- 1.5 meters) that made me suspect a projection issue rather than a data or computational issue.

image

It seems that the cell size precision in GRASS is slightly different than that of Whitebox, and in GRASS this seems to come from somewhere in src/r_grow_distance.py.

GRASS = 'transform': Affine(10.000000000000002, 0.0, -184570.59056412, 0.0, -10.000000000000014
Whitebox = 'transform': Affine(10.0, 0.0, -184570.5905641159, 0.0, -10.0, 1110604.2824222166)

Likewise, in QGIS the projection parameters are

GRASS
Origin -184570.5905641200079117,1110604.2824222201015800
Pixel Size 10.00000000000000178,-10.00000000000001421

Whitebox
Origin -184570.5905641159042716,1110604.2824222166091204
Pixel Size 10,-10

In both cases, the cell size becomes 10.0 in subsequent processing steps after AGREEDEM. This difference propagates to a difference in the REM (in this example, the difference between GRASS and Whitebox is over 2 meters):

image

The intermediate raster statistics between GRASS and Whitebox are otherwise identical in mean, standard deviation, etc., so perhaps Whitebox is correct given the exactly correct cell size. I don't know if this is the actual issue so it would be great if there was a GRASS expert who could see if we might be able to reconcile the two methods by correcting the GRASS projection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file FIM4 refactoring Refacting code to obtain the same result testing Evaluation and testing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants