Skip to content

Commit

Permalink
Merge branch 'main' of https://github.com/natcap/invest into bugfix/1…
Browse files Browse the repository at this point in the history
…350-una-cryptic-gdal-typeerror

Conflicts:
	HISTORY.rst
	tests/test_urban_nature_access.py
  • Loading branch information
phargogh committed Aug 29, 2023
2 parents 5829f4e + 9860be8 commit 87c9457
Show file tree
Hide file tree
Showing 86 changed files with 4,553 additions and 4,128 deletions.
7 changes: 4 additions & 3 deletions .github/actions/setup_env/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,14 @@ runs:
cat environment.yml
- name: Setup conda environment
uses: mamba-org/provision-with-micromamba@main
uses: mamba-org/setup-micromamba@v1
with:
environment-file: environment.yml
environment-name: env
channels: conda-forge
cache-env: true
cache-env-key: ${{ runner.os }}${{ runner.arch }}-${{ env.WEEK }}-${{ hashFiles('environment.yml') }}
init-shell: bash
cache-environment: true
cache-environment-key: ${{ runner.os }}${{ runner.arch }}-${{ env.WEEK }}-${{ hashFiles('environment.yml') }}

- name: List conda environment
shell: bash -l {0}
Expand Down
2 changes: 1 addition & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ Fixes #
## Checklist
- [ ] Updated HISTORY.rst and link to any relevant issue (if these changes are user-facing)
- [ ] Updated the user's guide (if needed)
- [ ] Tested the affected models' UIs (if relevant)
- [ ] Tested the Workbench UI (if relevant)
51 changes: 39 additions & 12 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ jobs:
run: make userguide

- name: Build binaries
run: make CONDA=micromamba binaries
run: make CONDA="$MAMBA_EXE" binaries

- name: Run invest-autotest with binaries
if : |
Expand All @@ -344,7 +344,18 @@ jobs:
yarn config set network-timeout 600000 -g
yarn install
- name: Build Workbench
- name: Authenticate GCP
if: github.event_name != 'pull_request'
uses: google-github-actions/auth@v0
with:
credentials_json: ${{ secrets.GOOGLE_SERVICE_ACC_KEY }}

- name: Set up GCP
if: github.event_name != 'pull_request'
uses: google-github-actions/setup-gcloud@v0

- name: Build Workbench (PRs)
if: github.event_name == 'pull_request'
working-directory: workbench
env:
GH_TOKEN: env.GITHUB_TOKEN
Expand All @@ -354,19 +365,35 @@ jobs:
yarn run build
yarn run dist
- name: Test electron app with puppeteer
- name: Build Workbench (macOS)
if: github.event_name != 'pull_request' && matrix.os == 'macos-latest' # secrets not available in PR
working-directory: workbench
run: npx cross-env CI=true yarn run test-electron-app
env:
GH_TOKEN: env.GITHUB_TOKEN
DEBUG: electron-builder
CSC_LINK: 2025-01-16-Expiry-AppStore-App.p12
CSC_KEY_PASSWORD: ${{ secrets.MACOS_CODESIGN_CERT_PASS }}
run: |
gsutil cp gs://stanford_cert/$CSC_LINK $CSC_LINK
yarn run build
yarn run dist
- name: Authenticate GCP
if: github.event_name != 'pull_request'
uses: google-github-actions/auth@v0
with:
credentials_json: ${{ secrets.GOOGLE_SERVICE_ACC_KEY }}
- name: Build Workbench (Windows)
if: github.event_name != 'pull_request' && matrix.os == 'windows-latest' # secrets not available in PR
working-directory: workbench
env:
GH_TOKEN: env.GITHUB_TOKEN
DEBUG: electron-builder
CSC_LINK: Stanford-natcap-code-signing-cert-expires-2024-01-26.p12
CSC_KEY_PASSWORD: ${{ secrets.WINDOWS_CODESIGN_CERT_PASS }}
run: |
gsutil cp gs://stanford_cert/$CSC_LINK $CSC_LINK
yarn run build
yarn run dist
- name: Set up GCP
if: github.event_name != 'pull_request'
uses: google-github-actions/setup-gcloud@v0
- name: Test electron app with puppeteer
working-directory: workbench
run: npx cross-env CI=true yarn run test-electron-app

- name: Sign binaries (macOS)
if: github.event_name != 'pull_request' && matrix.os == 'macos-latest' # secrets not available in PR
Expand Down
121 changes: 118 additions & 3 deletions HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -35,31 +35,86 @@

.. :changelog:
3.14.0 (YYYY-MM-DD)
-------------------
* SDR
* We implemented two major functional changes to the InVEST LS Factor
that significantly affect most outputs of SDR and will bring the LS
factor output more in line with the outputs of SAGA-GIS's LS Factor.
A discussion of differences between these two implementations can be
viewed at https://github.com/natcap/invest/tree/main/doc/decision-records/ADR-0001-Update-SDR-LS-Factor.md.
The two specific changes implemented are:

* The LS Factor's on-pixel aspect length is now calculated as
``abs(sin(slope)) + abs(cos(slope))``.
* The LS Factor's upstream contributing area is now calculated as
an estimate for the specific catchment area, calculated by
``sqrt(n_pixels_upstream * pixel_area)``.

Unreleased Changes
------------------
* General
* Fixed a bug in the CLI where ``invest getspec --json`` failed on
non-json-serializable objects such as ``pint.Unit``.
https://github.com/natcap/invest/issues/1280
* A new directory at `./doc/decision-records` has been created for
"Architecture/Any Decision Records", which will serve as a record of
nontrivial decisions that were made to InVEST and why. This is
intended for reference by our science and software teams, and also by
the community at large when inquiring about a nontrivial change.
https://github.com/natcap/invest/issues/1079
* Updated the package installation instructions in the API docs for clarity
and also to highlight the ease of installation through ``conda-forge``.
https://github.com/natcap/invest/issues/1256
* ``utils.build_lookup_from_csv`` has been deprecated and its functionality
has been merged into ``utils.read_csv_to_dataframe``
(`#1319 <https://github.com/natcap/invest/issues/1319>`_),
(`#1327 <https://github.com/natcap/invest/issues/1327>`_)
* Improved the validation message that is returned when not all spatial
inputs overlap (`#502 <https://github.com/natcap/invest/issues/502>`_)
* Standardized the name and location of the taskgraph cache directory for
all models. It is now called ``taskgraph_cache`` and located in the top
level of the workspace directory.
(`#1230 <https://github.com/natcap/invest/issues/1230>`_)
* Workbench
* Fixed a bug where sampledata downloads failed silently (and progress bar
became innacurate) if the Workbench did not have write permission to
the download location. https://github.com/natcap/invest/issues/1070
* The workbench app is now distributed with a valid code signature
(`#727 <https://github.com/natcap/invest/issues/727>`_)
* Changing the language setting will now cause the app to relaunch
(`#1168 <https://github.com/natcap/invest/issues/1168>`_)
* Closing the main window will now close any user's guide windows that are
open. Fixed a bug where the app could not be reopened after closing.
(`#1258 <https://github.com/natcap/invest/issues/1258>`_)
* Fixed a bug where invalid metadata for a recent run would result
in an uncaught exception.
(`#1286 <https://github.com/natcap/invest/issues/1286>`_)
* Middle clicking an InVEST model tab was opening a blank window. Now
middle clicking will close that tab as expected.
(`#1261 <https://github.com/natcap/invest/issues/1261>`_)
* Coastal Blue Carbon
* Added validation for the transition table, raising a validation error if
unexpected values are encountered.
(`#729 <https://github.com/natcap/invest/issues/729>`_)
* Forest Carbon
* The biophysical table is now case-insensitive.
* HRA
* Fixed a bug in HRA where the model would error when all exposure and
consequence criteria were skipped for a single habitat. The model now
correctly handles this case. https://github.com/natcap/invest/issues/1250
* Tables in the .xls format are no longer supported. This format was
deprecated by ``pandas``. (`#1271 <https://github.com/natcap/invest/issues/1271>`_)
deprecated by ``pandas``.
(`#1271 <https://github.com/natcap/invest/issues/1271>`_)
* Fixed a bug where vector inputs could be rasterized onto a grid that is
not exactly aligned with other raster inputs.
(`#1312 <https://github.com/natcap/invest/issues/1312>`_)
* NDR
* The contents of the output ``cache_dir`` have been consolidated into
``intermediate_outputs``.
* Fixed a bug where results were calculated incorrectly if the runoff proxy
raster (or the DEM or LULC) had no nodata value
(`#1005 <https://github.com/natcap/invest/issues/1005>`_)
* Pollination
* Several exceptions have been tidied up so that only fieldnames are
printed instead of the python data structures representing the whole
Expand All @@ -85,6 +140,8 @@ Unreleased Changes
* Fixed an issue with sediment deposition progress logging that was
causing the "percent complete" indicator to not progress linearly.
https://github.com/natcap/invest/issues/1262
* The contents of the output ``churn_dir_not_for_humans`` have been
consolidated into ``intermediate_outputs``.
* Seasonal Water Yield
* Fixed a bug where monthy quickflow nodata pixels were not being passed
on to the total quickflow raster, which could result in negative values
Expand All @@ -96,18 +153,76 @@ Unreleased Changes
set to 0. The old behavior was not well documented and caused some
confusion when nodata pixels did not line up. It's safer not to fill in
unknown data. (`#1317 <https://github.com/natcap/invest/issues/1317>`_)
* Negative monthly quickflow values will now be set to 0. This is because
very small negative values occasionally result from valid data, but they
should be interpreted as 0.
(`#1318 <https://github.com/natcap/invest/issues/1318>`_)
* In the monthly quickflow calculation, QF_im will be set to 0 on any pixel
where s_i / a_im > 100. This is done to avoid overflow errors when
calculating edge cases where the result would round down to 0 anyway.
(`#1318 <https://github.com/natcap/invest/issues/1318>`_)
* The contents of the output ``cache_dir`` have been consolidated into
``intermediate_outputs``.
* Urban Flood Risk
* Fixed a bug where the model incorrectly raised an error if the
biophysical table contained a row of all 0s.
(`#1123 <https://github.com/natcap/invest/issues/1123>`_)
* The contents of the output ``temp_working_dir_not_for_humans`` have been
consolidated into ``intermediate_files``.
* Biophysical table Workbench validation now warns if there is a missing
curve number value.
(`#1346 <https://github.com/natcap/invest/issues/1346>`_)
* Urban Nature Access
* Urban nature supply outputs have been renamed to add ``percapita`` to the
filename.

* In uniform search radius mode, ``urban_nature_supply.tif`` has been
renamed to ``urban_nature_supply_percapita.tif``.
* When defining search radii by urban nature class,
``urban_nature_supply_lucode_[LUCODE].tif`` has been renamed to
``urban_nature_supply_percapita_lucode_[LUCODE].tif``.
* When defining search radii by population groups,
``urban_nature_supply_to_[POP_GROUP].tif`` has been renamed to
``urban_nature_supply_percapita_to_[POP_GROUP].tif``.

* A new output for "Accessible Urban Nature" is created, indicating the
area of accessible greenspace available to people within the search
radius, weighted by the selected decay function. The outputs vary
slightly depending on the selected execution mode.

* In uniform search radius mode, a single new output is created,
``accessible_urban_nature.tif``.
* When defining search radii by urban nature class, one new
output raster is created for each class of urban nature. These files
are named ``accessible_urban_nature_lucode_[LUCODE].tif``.
* When defining search radii for population groups, one new output
raster is created for each population group. These files are named
``accessible_urban_nature_to_[POP_GROUP].tif``.

* Urban nature classes can now be defined to occupy a proportion of a
pixel, such as a park that is semi-developed. This proportion is
provided through user input as a proportion (0-1) in the
``urban_nature`` column of the LULC Attribute Table. A value of ``0``
indicates that there is no urban nature in this class, ``0.333``
indicates that a third of the area of this LULC class is urban nature,
and ``1`` would indicate that the entire LULC class's area is urban
nature. https://github.com/natcap/invest/issues/1180
* Fixed an issue where, under certain circumstances, the model would raise
a cryptic ``TypeError`` when creating the summary vector.
https://github.com/natcap/invest/issues/1350
* Visitation: Recreation and Tourism
* Fixed a bug where overlapping predictor polygons would be double-counted
in ``polygon_area_coverage`` and ``polygon_percent_coverage`` calculations.
(`#1310 <https://github.com/natcap/invest/issues/1310>`_)
in ``polygon_area_coverage`` and ``polygon_percent_coverage``
calculations. (`#1310 <https://github.com/natcap/invest/issues/1310>`_)
* Changed the calculation of ``point_nearest_distance`` metric to match
the description in the User's Guide. Values are now the distance to the
centroid of the AOI polygon instead of the distance to the nearest
edge of the AOI polygon.
(`#1347 <https://github.com/natcap/invest/issues/1347>`_)
* Wind Energy
* Updated a misleading error message that is raised when the AOI does
not spatially overlap another input.
(`#1054 <https://github.com/natcap/invest/issues/1054>`_)

3.13.0 (2023-03-17)
-------------------
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
DATA_DIR := data
GIT_SAMPLE_DATA_REPO := https://bitbucket.org/natcap/invest-sample-data.git
GIT_SAMPLE_DATA_REPO_PATH := $(DATA_DIR)/invest-sample-data
GIT_SAMPLE_DATA_REPO_REV := a58b9c7bdd8a31cab469ea919fe0ebf23a6c668e
GIT_SAMPLE_DATA_REPO_REV := 2e7cd618c661ec3f3b2a3bddfd2ce7d4704abc05

GIT_TEST_DATA_REPO := https://bitbucket.org/natcap/invest-test-data.git
GIT_TEST_DATA_REPO_PATH := $(DATA_DIR)/invest-test-data
GIT_TEST_DATA_REPO_REV := a89253d83d5f70a8ea2d8a951b2d47d603505f14
GIT_TEST_DATA_REPO_REV := e7d32d65612f4f3578a4fb57824af4e297c65283

GIT_UG_REPO := https://github.com/natcap/invest.users-guide
GIT_UG_REPO_PATH := doc/users-guide
Expand Down
94 changes: 94 additions & 0 deletions doc/decision-records/ADR-0001-Update-SDR-LS-Factor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# ADR-0001: Update the InVEST SDR LS Factor

Author: James

Science Lead: Rafa

## Context

Since we released the updated InVEST SDR model in InVEST 3.1.0, we have seen a
common refrain of users and NatCap science staff noticing that the LS factor
output of SDR did not produce realistic results and that the LS factor produced
by SAGA was much more realistic. We have over the years made a couple of notable
changes to the model and to the LS factor that have altered the output including:

1. The SDR model's underlying routing model was changed from d-infinity to MFD in 3.5.0
2. The $x$ parameter was changed in InVEST 3.8.1 from the true on-pixel aspect
$|\sin \theta|+|\cos \theta|$ (described in Zevenbergen & Thorne 1987 and repeated
in Desmet & Govers 1996) to the weighted mean of proportional flow from the
current pixel to its neighbors.
3. A typo in a constant value in the LS factor was corrected in InVEST 3.9.1
4. An `l_max` parameter was exposed to the user in InVEST 3.9.1

Despite these changes to the LS factor, we still received occasional reports
describing unrealistic LS factor outputs from SDR and that SAGA's LS factor
was much more realistic.

After diving into the SAGA source code, it turns out that there are several
important differences between the two despite both using Desmet & Govers (1996)
for their LS factor equations:

1. The contributing area $A_{i,j-in}$ is not strictly defined in Desmet &
Govers (1996), it is only referred to as "the contributing area at the inlet
of a grid cell with coordinates (i, j) (m^2)".
InVEST assumes that "contributing area" is $area_{pixel} \cdot n\\_upstream\\_pixels$.
SAGA refers to this as "specific catchment area" and allows the user to choose their
specific catchment area equation, where the available options are
"contour length simply as cell size", "contour length dependent on aspect", "square
root of catchment area" and "effective flow length".
2. SAGA uses on-pixel aspect, $|\sin \theta|+|\cos \theta|$, and does not consider
flow direction derived from a routing model when calculating the LS factor.
3. The length exponent $m$ differs between the implementations. In SAGA,
$m = \beta / (1 + \beta)$. In InVEST, we have a discontinuous function where
$m$ is dependent on the slope of the current pixel and described as "classical USLE"
in the user's guide and discussed in Oliveira et al (2013).
4. SAGA's flow accumulation function [`Get_Flow()`](https://github.com/saga-gis/saga-gis/blob/master/saga-gis/src/tools/terrain_analysis/ta_hydrology/Erosion_LS_Fields.cpp#L394)
only considers a pixel downstream if and only if its elevation is strictly less
than the current pixel's elevation, which implies that flow accumulation will
not navigate plateaus. InVEST's flow accumulation handles plateaus well,
which can lead to longer flow accumulation values on the same DEM.
5. SAGA's flow accumulation function `Get_Flow()` uses D8, InVEST's flow
accumulation uses MFD.

It is important to note that when evaluating differences between the SAGA and InVEST
LS Factor implementations, it is _critical_ to use a hydrologically conditioned DEM such
as conditioned by Wang & Liu so that we control for differences in output due
to the presence of plateaus.

Once we finally understood these discrepancies, James implemented several of the
contributing area functions available in SAGA to see what might be most comparable
to the real world. Source code and a docker container for these experiments are
available at
https://github.com/phargogh/invest-ls-factor-vs-saga/blob/main/src/natcap/invest/sdr/sdr.py#L901.
Some additional discussion and notes can be viewed in the related github issue:
https://github.com/natcap/invest/issues/915.

## Decision

After inspecting the results, Rafa decided that we should make these changes to
the LS Factor calculation:

1. We will revert to using the on-pixel aspect, $|\sin \theta|+|\cos \theta|$.
This is in line with the published literature.
2. We will convert the "contributing area" portion of the LS Factor to be
$\sqrt{ n\\_upstream\\_pixels \cdot area\_{pixel} }$. Rafa's opinion on this
is that the LS factor equations were designed for a 1-dimensional situation,
so our specific catchment area number should reflect this.

## Status

## Consequences

Once implemented and released, the LS factor outputs of SDR will be
significantly different, but they should more closely match reality.

We hope that there will be fewer support requests about this once the change is
released.

## References

Zevenbergen & Thorne (1987): https://searchworks.stanford.edu/articles/edb__89861226

Desmet & Govers (1996): https://searchworks.stanford.edu/articles/edsgac__edsgac.A18832564

Oliveira et al (2013): http://dx.doi.org/10.5772/54439
12 changes: 12 additions & 0 deletions doc/decision-records/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Architecture/Any Decision Records

An ADR is a way to track decisions and their rationale in a way that is tied to
the source code, easy to digest, and written in a way that future us will
understand. An ADR consists of several sections:

1. The title and ADR number (for easier sorting)
2. Context about the problem
3. The decision that was made and why
4. The status of implementation
5. Consequences of the implementation
6. Any references (especially if describing a science/software issue)
Loading

0 comments on commit 87c9457

Please sign in to comment.