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

UNA: Population group SUP_DEM outputs missing in admin gpkg #1531

Closed
newtpatrol opened this issue Feb 21, 2024 · 6 comments
Closed

UNA: Population group SUP_DEM outputs missing in admin gpkg #1531

newtpatrol opened this issue Feb 21, 2024 · 6 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@newtpatrol
Copy link

I'm running 3.14.1 with the Aggregate by Population Groups option (not radii by population group). It runs successfully, and according to the User Guide, there should be 3 fields added to admin_boundaries.gpkg corresponding with each population group. Pund_adm and Povr_adm are added for each population group, but SUP_DEMadm_cap is not added per population group, the only SUP_DEM column is for the total population. This seems like a bug.

Here's the datastack I'm using that produces this result (bummer that GitHub doesn't support .tgzs for direct upload).

@davemfish davemfish added this to the 3.14.2 milestone Feb 21, 2024
@emlys emlys changed the title Population group SUP_DEM outputs missing in admin gpkg UNA: Population group SUP_DEM outputs missing in admin gpkg Apr 24, 2024
@emlys emlys self-assigned this Apr 24, 2024
@emlys emlys added the bug Something isn't working label Apr 24, 2024
@emlys
Copy link
Member

emlys commented Apr 24, 2024

Confirmed this is a bug.

@emlys
Copy link
Member

emlys commented Apr 25, 2024

After spending all day trying to understand the different UNA modes, I'm still not totally sure what the answer is.

As far as I can tell, the model design doc does not specify a SUP_DEM result per population group when run in "aggregate by population groups" mode.

This kind of makes sense, because the SUP_DEM per capita results would be the same for all groups:

  • Urban nature supply per capita (The distance-weighted sum of urban nature supply per capita within the search radius of each pixel) is different between population groups IF those population groups have different search radii.
  • When run WITHOUT "radii by population group" mode, each population group has the same search radius.
  • Therefore, urban nature supply per capita is the same for all groups.
  • SUP_DEM results are calculated as the difference between urban nature supply (per capita) and urban nature demand (per capita). ($A_i - g_{cap}$ in the users guide). Urban nature demand is a constant that applies to all groups.
  • Therefore, supply and demand are constant across all population groups when run WITHOUT "radii by population group" mode. SUP_DEM results would be the same across all population groups, so there is no need to report them for each group.

Intuitively, because everyone has the same search radius, the urban nature supply and demand is the same for everyone living on a given pixel.

But the people living on each pixel may disproportionately belong to one group or another. Thus, each group may disproportionately experience under- or over-supply of urban nature. We report that as $P_{und}$ and $P_{ovr}$ (units: number of people).

Conceptually, I think you could report a total SUP_DEM by population group (units: m^2).
We already have

  • Urban nature balance per capita (m^2 per person): $SUPDEM_{i,cap} = A_i - g_{cap}$
  • Urban nature balance (m^2): $SUPDEM_{i} = SUPDEM_{i,cap} * P_i$

So I think you could also do

  • Urban nature balance (m^2) for group $g$: $SUPDEM_{i,g} = SUPDEM_{i,cap} * P_{i,g}$

But that is not part of the original specification, and I'm not sure that it adds much value to an already confusing model.

@phargogh
Copy link
Member

phargogh commented May 9, 2024

I have been going back and forth on this, but at this point I wonder if it might just be easier to implement the as-yet-unimplemented trivial calculations for urban nature balance for pop groups when aggregating by pop groups even though it doesn't add much interesting information.

The root bug here is that I, for some reason I don't recall, thoughy it would be a good idea to allow the user to aggregate by population groups when not running the model with search radii defined per population group.

W/r/t what to do about this issue, I agree that this model is ridiculously complicated for how simple its operations are, but I also wonder about the time required to make the model fully compliant with the original spec.

@emlys would you agree that a code patch for this would be as simple as the below (not counting needed changes in the UG)?

diff --git a/src/natcap/invest/urban_nature_access.py b/src/natcap/invest/urban_nature_access.py
index 2831e6528..5f3b2345e 100644
--- a/src/natcap/invest/urban_nature_access.py
+++ b/src/natcap/invest/urban_nature_access.py
@@ -2074,6 +2074,8 @@ def _supply_demand_vector_for_single_raster_modes(
                                          ('Povr', oversupplied_stats)]:
                 stats[f'{prefix}_adm_{group}'] = (
                     supply_stats[fid]['sum'] * group_proportion)
+            stats[f'SUP_DEMadm_cap_{group}'] = (
+                per_capita_supply * group_proportion)
         stats_by_feature[fid] = stats

     _write_supply_demand_vector(

@newtpatrol
Copy link
Author

newtpatrol commented May 9, 2024

For what it's worth, seeing @emlys very helpful evaluation, and thinking about it more, I agree that it doesn't make sense to include a SUP_DEM result per population group when not running with different radii per population group, since the result is per capita, and will be the same as the general SUP_DEM result.

I do think it's good to provide the over- and under-supply by population group, since those will change based on the proportion of different groups, even without differing radii, and definitely provides useful information related to equity.

Maybe I'm over-simplifying, but my suggestion is not to add any other model outputs, but just update the User Guide to correspond with the current outputs, and add explanatory text about SUP_DEM being the same in this case. I'd be happy to make these updates if that's helpful.

@emlys
Copy link
Member

emlys commented May 21, 2024

@newtpatrol we discussed this today on the software team call and agree with leaving the model as-is, and just updating the user guide.

@phargogh phargogh self-assigned this May 23, 2024
phargogh added a commit to natcap/invest.users-guide that referenced this issue May 23, 2024
@phargogh
Copy link
Member

The UG has been updated to address this issue, so I believe we can now close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants