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

Manual Entry: Recalculate Area-derived Values on Land Cover Updates #3151

Merged
merged 3 commits into from Aug 27, 2019

Conversation

@rajadain
Copy link
Member

rajadain commented Aug 26, 2019

Overview

MapShed operates on a large dictionary of values, whose fields can have static, lookup, user-provided, or calculated values. A number of these values are calculated using rasters geoprocessed for the given area of interest. One of these rasters in the NLCD raster, which is provides a count of cells for each land cover type. These counts are stored in the Area key, but are also used to calculate some derived values. The final dictionary with all the values is saved to the gis_data field of the project.

As of #2977, users can override the land cover counts for a given scenario. This modification is saved on the scenario. When running MapShed, these modifications are combined with the gis_data of the project to produce a scenario-specific dictionary, whose values are used to run GWLF-E and get results.

Unfortunately, so far we have been simply overriding the Area key of gis_data, and not any of the other keys which derive their value from the Area list. This is partially addressed in this PR.

Keys whose values are affected by the Area list are extracted into an area_calculations function which is called from collect_data, and also from apply_gwlfe_modifications, to ensure that their values are correctly derived on initialization and update.

This is done on the back-end for facilitating improved GWLF-E calculations, and on the front-end for allowing Land Cover modifications to inform Conservation Practice modifications.

See commit messages for details.

Connects #3142

Demo

$ ./scripts/manage.sh test_mapshed
+ ARGS=test_mapshed
+ [[ 1 -eq 1 ]]
+ [[ test_mapshed == runserver ]]
+ vagrant ssh app -c 'cd /opt/app && envdir /etc/mmw.d/env ./manage.py test_mapshed'
.
----------------------------------------------------------------------
Ran 1 test in 1.124s

OK
Connection to 127.0.0.1 closed.

If the land use is overridden:

localhost_8000_project_13_scenario_17_

The conservation practice also updates:

localhost_8000_project_13_scenario_17_ (1)

Notes

There are two exceptions to the fields updated, one overt and one subtle:

Despite being computed from Area, the value of LS is not recalculated, because it also relies on the lu_stream_pct geoprocessing result. This is not currently not saved, and thus cannot be used during recalculation. Thus, this field is left to its initially calculated value.

A number of other geoprocessing operations combine NLCD with another raster to get a grouped count, such as nlcd_soil, nlcd_streams, nlcd_slope, and nlcd_kfactor. These are in turn used to calculate a number of results, such as cn, ag_stream_pct, low_urban_stream_pct, med_high_urban_stream_pct, lu_stream_pct, ag_slope_3_pct, ag_slope_3_8_pct, n41, kf, and avg_kf. These are in turn used to calculate a number of MapShed fields, such as AgLength, UrbLength, n42, n46e, n46f, CN, AgSlope3, AgSlope3To8, n41, AvKF, KF, and P.

Recalculating these fields from the modifications is complicated, because the modifications are only changing the proportion of land use distribution, not the actual distribution in space. Without the spatial updates, we cannot run raster operations. This will need to be clarified and addressed at a later date.

Additionally, there are some concerns outlined here about the implications of modifications affecting each other.

Testing Instructions

  • Check out this branch and reprovision app

    $ vagrant reload app --provision
  • Check out only the first commit, which adds the test but doesn't contain the refactoring, and run the test:

    $ git checkout HEAD~2
    $ ./scripts/manage.sh test_mapshed
  • Check out the main branch, and run the test again to ensure it runs correctly:

    $ git checkout -
    $ ./scripts/manage.sh test_mapshed
  • Go to :8000/ and create a MapShed project

  • Add changes to Current Conditions

  • Open the Land Cover modal

  • Change the area of Cropland. Change the area of something else to compensate. Save your changes.

  • Try and add a conservation practice, such as Cover Crops

    • Ensure the Area of Row Crops matches your user input value
  • Go back to the Land Cover modal and reset to original values

  • Try and add the same conservation practice again

    • Ensure the Area of Row Crops matches the original value

Checklist

  • All JavaScript tests pass ./scripts/testem.sh
@hectcastro hectcastro changed the base branch from develop to release/1.25.0 Aug 26, 2019
@rajadain

This comment has been minimized.

Copy link
Member Author

rajadain commented Aug 26, 2019

@azavea-bot rebuild

rajadain added 3 commits Aug 19, 2019
Using a sample area of interes and observed output, add
a test that ensures the output matches. This is useful
when making changes to the calculation to ensure that
the numbers do not change significantly when the calcs
are updated.

The dictdiffer package is brought in which allows for
deep comparison of dicts, with a configurable tolerance
for numeric comparison. The tests are currently set to
tolerate differences smaller than 1e-15.

The test is implemented as a management command rather
than a unit test because it needs access to the special
MapShed tables to run the calculations, which are not
available in the Unit Test database. To add the large
tables as fixtures to the Unit Test database would be
too big of a slowdown for the sake of this one test,
and would considerably bloat the repo size. To ensure
the test does not run with the others, it is tagged and
excluded from the test script.
Previously, when the user would override Land Use values,
we would only update the Area list in the MapShed dict.
However, there are a number of other fields in the dict
whose values are derived from the Area list, which were
not updated.

The derivation of those fields is now extracted into its
own method, which is called in the initialization of the
dict, as well as during ingestion of modifications from
the user. If the set of modifications contains updates
to the land use distribution, then the area derived
fields are recalculated.

Notably, the LS calculation is not included in this
refactor. This is because while it does derive its value
from Area, it is also dependent on the geoprocessing
result of `lu_stream_pct`, which isn't saved anywhere.
Because of that, we cannot adjust its value with new
proportions when the user updates land use changes.
Amending this would require a larger refactor, which is
deferred to a later date.

The test introduced in the previous commit serves to
ensure that this refactoring does not alter the
calculations.
The changes made in the previous commit recalculate the
derived values for running the MapShed model. The changes
in this commit recalculate them for the data model in the
front-end, which allows for other modifications to pick up
on the land use changes.

For example, if the user changes the area of Cropland in
the Land Cover modal, the Area of Row Crops should change
correspondingly for the respective Conservation Practices.
This will now happen, as `n23` (the model field representing
the Area of Row Crops), amongst others, is now updated.
@rajadain rajadain force-pushed the tt/manual-entry-propagate-land-use-changes branch from abaef71 to baccd9f Aug 26, 2019
@rajadain

This comment has been minimized.

Copy link
Member Author

rajadain commented Aug 26, 2019

@azavea-bot rebuild

Copy link
Member

caseycesari left a comment

Working as described! I don't quite follow all the details because I don't have a deep understanding of mapshed, but your overview was helpful and made sense to me.

@caseycesari caseycesari assigned rajadain and unassigned caseycesari Aug 26, 2019
@rajadain

This comment has been minimized.

Copy link
Member Author

rajadain commented Aug 27, 2019

Thanks for taking a look! I'm going to merge this, and make some follow up enhancement issues for the ideas covered in Notes.

@rajadain rajadain merged commit 0bffdc8 into release/1.25.0 Aug 27, 2019
2 checks passed
2 checks passed
default Build finished.
Details
model-my-watershed-pull-requests Build #4091 succeeded in 9 min 0 sec
Details
@rajadain rajadain deleted the tt/manual-entry-propagate-land-use-changes branch Aug 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.