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

Allow Projects to Override Layers for Geoprocessing #3411

Merged
merged 4 commits into from
Aug 5, 2021

Conversation

rajadain
Copy link
Member

@rajadain rajadain commented Jul 29, 2021

Overview

Previously, every layer class (land, soil, etc.) had a single raster layer for it. As of recent work done in #3396 and #3399, we now have allow multiple raster layers for ever class of raster. The actual raster used is specified in the settings, can be overridden, as of #3406.

This PR adds override capability to Projects, in the form of a layer_overrides JSON field. The value of this field will be used to override the default layer configuration on every geoprocessing run. For all existing projects, this is set to override the land layer with NLCD 2011. For all new projects, no overrides are set. The front-end is updated to read this value from the project and use it when firing model runs, and the back-end is updated to expect this value in the model runs and use it. Finally, the default land layer is set to NLCD 2019.

This allows us to preserve the work done in all existing projects, thus not changing results from under our users without their awareness, while still using the latest data for all work going forward.

Connects #3400

Testing Instructions

Much of the work in this PR is to preserve existing data. Thus, in order to test this, you must create local saved work on develop, then checkout this branch and perform the upgrades, then create some new work, and ensure that the old work is preserved.

To compare old and new calculations, we'll use the same Area of Interest in both cases.

  • Start on the latest develop
  • Ensure you locally have a user and are logged in
    • In case registering through the app, follow the Django log to see the confirmation link in the email:
      $ vagrant ssh app -c 'sudo journalctl -fu mmw-app'
  • Find an Area of Interest that has noticeable changes between 2011 and 2019
    • I used the CONUS Land Cover Change Index layer in https://www.mrlc.gov/viewer/ to find Flat Rock Creek near Kenbridge, VA that works well. You can search for Kenbridge in the geocoder, go there, then click "Select Boundary", "HUC-12", then "Flat Rock Creek":
      2021-07-28 21 33 38
  • Once analysis is done, go to the Model tab and select "Site Storm Model". Since you are logged in, this project will be saved to the database. Rename the project to be "TR55 Old 2011"
  • Under "Details", click "Edit" next to Model to go back one step to Analyze
    image
  • Next, select the "Watershed Multi-Year Model". Rename this project to be "MapShed Old 2011"

Now that our reference projects are setup, we can start testing the work in this branch.

  • Checkout this branch and run ./scripts/manage.sh migrate and ./scripts/bundle.sh --debug
  • Clear your geoprocessing cache vagrant ssh services -c 'redis-cli -n 1 --raw KEYS ":1:geop_*" | xargs redis-cli -n 1 DEL'
  • Restart Celery vagrant ssh worker -c 'sudo service celeryd restart'
  • Go to http://localhost:8000/ and open the Network tab of developer tools
  • Select the same Area of Interest and create a Site Storm Model project named "TR55 New 2019"
  • With the same Area of Interest, create a Watershed Multi-Year Model project named "MapShed New 2019"
  • Go to your projects (from the top-right), and open the old projects (perhaps in new tabs)
  • Compare the output numbers in the tables between the old and new projects
    • Ensure they are different
  • In the TR55 Old 2011 project, with the Network tab open, create a new Scenario by clicking "Add changes to this project"
  • In the New Scenario, change the Precipitation slider, to re-run the TR-55 model
  • In the Network tab, notice that the payload for POST http://localhost:8000/mmw/modeling/tr55/ contains a layer_overrides: {"__LAND__":"nlcd-2011-30m-epsg5070-512-int8"} key
    image
  • Inspect the database with ./scripts/manage.sh dbshell
    • Ensure that old projects have the 2011 override set, and the new ones don't:
      SELECT id, name, layer_overrides FROM modeling_project;
      
       id |       name       |                 layer_overrides                 
      ----+------------------+-------------------------------------------------
        1 | TR55 Old 2011    | {"__LAND__": "nlcd-2011-30m-epsg5070-512-int8"}
        2 | MapShed Old 2011 | {"__LAND__": "nlcd-2011-30m-epsg5070-512-int8"}
        9 | TR55 New 2019    | {}
       10 | MapShed New 2019 | {}
      (4 rows)

This field will be used to save the layer overrides for each
project. All existing projects are initialized with NLCD 2011,
and all new projects will be initialized with an empty override
dictionary.

We use a JSONField which is available as of Django 1.11. This
is a better representation than TextField which is what was used
so far.
When the front-end requests a model run, we now include the
layer_overrides in the model input. This is done for TR-55,
MapShed, and Subbasin.
@rajadain rajadain added the PA DEP Funding Source: Pennsylvania Department of Environment Protection label Jul 29, 2021
@rajadain rajadain requested a review from jwalgran July 29, 2021 02:15
Copy link
Member Author

@rajadain rajadain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments. The rest of the diff is just wiring layer_overrides through all the necessary parts.

Comment on lines +19 to +20
field=django.contrib.postgres.fields.jsonb.JSONField(default={'__LAND__': 'nlcd-2011-30m-epsg5070-512-int8'}, help_text='JSON object of layers to override defaults with'),
preserve_default=False,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures that the 2011 override is used for all existing projects, but no longer.

migrations.AlterField(
model_name='project',
name='layer_overrides',
field=django.contrib.postgres.fields.jsonb.JSONField(default=dict, help_text='JSON object of layers to override defaults with'),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures that the default is an empty dictionary, effectively no overrides, for all new projects.

@jwalgran
Copy link
Contributor

I have started reviewing this, but am switching to pair on another project.

@jwalgran
Copy link
Contributor

When setting up the test on the develop branch I saw some 500 errors in the network tab related to weather stations.

Screen Shot 2021-07-30 at 2 02 41 PM

Because the error is a missing Postgres function I am assuming that my database VM may be outdated. I don't know if having a broken weather data endpoint will affect the testing of this feature.

@rajadain
Copy link
Member Author

rajadain commented Aug 2, 2021

Hmm, jsonb was added in Postgres 9.4, and to_jsonb was added in 9.5.

It's possible that reprovisioning your services VM is enough:

$ vagrant reload services --provision

But in case that doesn't work, you'll have to recreate it, which is a bit more time taking:

$ vagrant destroy services
$ vagrant up services

Then, edit your setupdb.sh to remove the tile purging bits (which shouldn't run locally):

diff --git a/scripts/aws/setupdb.sh b/scripts/aws/setupdb.sh
index 0d5b8443..4644c371 100755
--- a/scripts/aws/setupdb.sh
+++ b/scripts/aws/setupdb.sh
@@ -81,9 +81,7 @@ function download_and_load {
 }
 
 function purge_tile_cache {
-    for path in "${PATHS[@]}"; do
-        aws s3 rm --recursive "s3://tile-cache.${PUBLIC_HOSTED_ZONE_NAME}/${path}/"
-    done
+    echo "Skipping purging tile cache locally"
 }
 
 function create_trgm_indexes {

and then run:

$ vagrant ssh app -c 'cd /vagrant && ./scripts/aws/setupdb.sh -bsdpmqc'

to load all the necessary dev data. Then you'll have to create another account for yourself in the app, and then should be ready to go.


The DRB 2011 analyses may still fail because they need an API key set. But they are not necessary for testing this work.

@jwalgran
Copy link
Contributor

jwalgran commented Aug 4, 2021

I've been unable to complete testing and will need to schedule some pairing time to get this wrapped up. While retesting the process I noticed that the initial POST to create the streams job was timing out and returning 504.

Screen Shot 2021-08-03 at 7 53 41 PM

@rajadain
Copy link
Member Author

rajadain commented Aug 4, 2021

Let's pair on this today.

@@ -494,19 +494,21 @@ def nlcd_kfactor(result):
return output


def multi_mapshed(aoi, wkaoi):
def multi_mapshed(aoi, wkaoi, layer_overrides={}):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps I should replace this with dict

Suggested change
def multi_mapshed(aoi, wkaoi, layer_overrides={}):
def multi_mapshed(aoi, wkaoi, layer_overrides=dict):

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!



def multi_subbasin(parent_aoi, child_shapes):
def multi_subbasin(parent_aoi, child_shapes, layer_overrides={}):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too

Suggested change
def multi_subbasin(parent_aoi, child_shapes, layer_overrides={}):
def multi_subbasin(parent_aoi, child_shapes, layer_overrides=dict):

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!

Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me.

I spent so much time working on my dev environment I did not go back and rerun the entire test process after fixing my analysis/data issues. I compared new projects in development with staging and verified that they did produce different results.

Screen Shot 2021-08-04 at 12 14 23 PM

Screen Shot 2021-08-04 at 12 16 01 PM

@jwalgran
Copy link
Contributor

jwalgran commented Aug 4, 2021

We traced the 5xx timeout error I was seeing to the fact that my nhdflowlines table had no indexes. We got analysis working on my dev VM by loading/reloading layers.

Now that we've specified all existing projects to use the old
NLCD 2011 layer, we switch the default in the app to be the
NLCD 2019 layer.
@rajadain rajadain force-pushed the tt/add-dynamic-layer-support branch from 7bc8353 to 39266d8 Compare August 5, 2021 12:31
@rajadain rajadain merged commit 0f1c01e into develop Aug 5, 2021
@rajadain rajadain deleted the tt/add-dynamic-layer-support branch August 5, 2021 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PA DEP Funding Source: Pennsylvania Department of Environment Protection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants