-
Notifications
You must be signed in to change notification settings - Fork 31
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 Mass Restoration of TR-55 Projects #3530
Conversation
d93dcb8
to
2f62bf6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments.
@@ -525,6 +525,19 @@ def get_env_setting(setting): | |||
'zoom': 0 | |||
} | |||
}, | |||
'nlcd_soil_tr55': { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decided to make a new one instead of changing the existing one because I wasn't sure if that was used elsewhere.
if 'error' in results: | ||
raise Exception(f'[nlcd_soil_tr55] {results["error"]}') | ||
|
||
return [nlcd_soil(result) for result in results] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a simple wrapper around the old one, which runs it for every result
in the output of RasterGroupedCountMany.
@@ -44,7 +44,7 @@ docker_compose_version: "1.26.*" | |||
geop_host: "localhost" | |||
geop_port: 8090 | |||
|
|||
geop_version: "5.2.0" | |||
geop_version: "5.3.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may cause build failures until 5.3.0 is tagged and released for mmw-geoprocessing.
modification_hash='' | ||
modification_hash='', | ||
aoi_census='{}', | ||
modification_censuses='{}', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These need to be cleared as well for the scenarios to correctly re-run.
Since we're switching out the land cover layer, any cached censuses are invalid, and should be cleared out as well.
This ensures that multiple shapes can be analyzed at once, and their histograms are reported individually, rather than as a group, as was the case with RasterGroupedCount.
This operation takes multiple polygons and returns histograms for each of them. This is needed for TR-55, where multiple spatial modifications can exist on top of the area of interest, and need their own histograms.
2f62bf6
to
4b022a9
Compare
I've added 4b022a9 to get the CI to pass until the 5.3.0 release is available. Once it is, we should drop that commit before merging. |
4b022a9
to
153d9b9
Compare
I just released 5.3.0: https://github.com/WikiWatershed/mmw-geoprocessing/releases/tag/5.3.0. Going to remove that final commit. Also, with this new release being available, all the steps in the Preparation section can be replaced with this one line command: vagrant provision worker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the fix between master
and this branch. All modifications applied to the shape works well. Nice work!
Thank you! Merging this now, will test on staging shortly. |
Overview
We realized that the migration in #3528 did not fix the issue it was attempting to. This was because while we cleared the
results
andmodification_hash
fields, theaoi_census
andmodification_censuses
were not, and with their cached values continued to have the same issue as before.This PR includes an updated version of that migration that also clears those fields. Unfortunately, once those are gone, we realized that the code goes down a path not often trod:
model-my-watershed/src/mmw/apps/modeling/views.py
Lines 645 to 656 in afebb07
This code path is only exercised when neither the AoI census nor the modification censuses are defined, which almost never happens in regular user interaction. Usually, the AoI census is calculated when the Current Conditions scenario is initialized, and then copied over to New Scenarios. When adding modifications, only one is added at a time, and previously cached modification censuses are used otherwise. This is important because the current geoprocessing service only works on one polygon at a time, and if multiple polygons are provided, it unifies them into a single one and returns a combined result. The TR-55 code does expect multiple items, one for each polygon:
model-my-watershed/src/mmw/apps/modeling/tasks.py
Lines 254 to 259 in afebb07
but our geoprocessing summary returned the single result wrapped in a simple list:
model-my-watershed/src/mmw/apps/modeling/tasks.py
Lines 225 to 228 in afebb07
which works correctly as long as only a single polygon is queried. However, when multiple polygons are queried, the result is a combined histogram, whereas we want individualized ones.
Hence, in WikiWatershed/mmw-geoprocessing#101, we add a new operation
RasterGroupedCountMany
, which will be included in the upcoming 5.3.0 release of the geoprocessing service, that returns individualized histograms for each input polygon.This PR switches TR-55 to use that new operation.
Notes
While this should be tested ASAP, this cannot be merged until WikiWatershed/mmw-geoprocessing#101 is merged and a
5.3.0
version of the geoprocessing service is tagged and released.Testing Instructions
Preparation
The following is no longer needed as of the 5.3.0 release.
In a local install ofmmw-geoprocessing
, check out Add new operation supporting many input polygons mmw-geoprocessing#101, then run./scripts/cibuild
Copy the generated JAR file to yourmodel-my-watershed
directory:Go into yourworker
VM, and replace the local JAR with this one:sudo mv /vagrant/api-assembly-5.2.0.jar /opt/geoprocessing/mmw-geoprocessing-5.2.0.jar sudo service mmw-geoprocessing stop && sudo service mmw-geoprocessing start
Instead, just reprovision the
worker
to get the latest mmw-geoprocessing JAR:Evaluation
master
, ensure you are logged in so that your work is savedvagrant ssh worker -c 'sudo service celeryd restart'