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

feature abundance tiles [MRXN23-193] #1490

Merged
merged 5 commits into from
Sep 19, 2023

Conversation

aagm
Copy link
Member

@aagm aagm commented Sep 8, 2023

Ammounts at scenario level, this works for all features.

Overview

_Proposed solution that has been proven as valid:

image
Files touched to produce the tiles:
api/apps/geoprocessing/src/modules/scenario-planning-units-features-aggregate/scenario-planning-units-features-aggregate-processor.ts
We use the amounts stored in geodb puvspr_calculations
table that is created by merging the information on spatial features and legacy features/csv uploaded ones.

In the vector layer the ammounts are serve through the featureList property under the planning-units/tiles/{z}/{x}/{y}.mvt?include=features
in the form of featureList:"<featureid>:<ammount>;<featureid>:<ammount>;"
We will required to store the amount min/max at features and project levels (in a similar way as we will do for cost surface) and provide an endpoint or provide such information in the feature list call used in the pannel.

On the FE side of things the layer styles needs to incorporate this logic:

filter: ['all', ['in', id, ['get', 'featureList']]],
paint: {
  'fill-outline-color': 'yellow',
  'fill-color': [
    'let',
    'ammount',
    [
      'to-number',
      [
        'let',
        'idx',
        ['index-of', id, ['get', 'featureList']],
        [
          'slice',
          ['get', 'featureList'],
          [
            '+',
            [
              'index-of',
              ':',
              ['get', 'featureList'],
              ['var', 'idx'],
            ],
            1,
          ],
          [
            'index-of',
            ';',
            ['get', 'featureList'],
            ['var', 'idx'],
          ],
        ],
      ],
    ],
    [
      'interpolate',
      ['linear'],
      ['var', 'ammount'],
      1,
      'white',
      50000,
      'red',
      100000000,
      'green',
    ],
  ],
  'fill-opacity': 1,
}, 

Where the interpolation bit will depend on the min/max for each feature

Feature relevant tickets

  • MRXN23-203
  • MRXN23-193
  • MRXN23-195

Checklist before submitting

  • Meaningful commits and code rebased on develop.
  • If this PR adds feature that should be tested for regressions when
    deploying to staging/production, please add brief testing instructions
    to the deploy checklist (docs/deployment-checklist.md)
  • Update CHANGELOG file

@vercel
Copy link

vercel bot commented Sep 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marxan ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 19, 2023 11:20am

@aagm aagm requested a review from hotzevzl September 8, 2023 09:24
@aagm aagm added API Everything related the api Ready to review PR ready to review labels Sep 8, 2023
@hotzevzl hotzevzl changed the title Api/feature/mrxn23 203 abundance tiles feature abundance tiles [MRXN23-203] Sep 8, 2023
@hotzevzl hotzevzl changed the title feature abundance tiles [MRXN23-203] feature abundance tiles [MRXN23-193] Sep 11, 2023
@hotzevzl
Copy link
Member

@aagm thank you. I will need a small walkthrough as I haven't actually been able to get this PR to work as I expected: I think I am simply misunderstanding where I should be looking for this data.

I have created test projects, and either used features from shapefiles, or uploaded features from puvspr data (csv), selected some features for a new scenario, and saved the feature specification. Once this is done, however, scenarios_pu_data.feature_list ends up always being an empty array (and consequently the MVTs I download have empty featureList attributes), which is why I think I am holding this PR wrongly to start with 😬

Copy link
Member

@hotzevzl hotzevzl left a comment

Choose a reason for hiding this comment

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

thanks @aagm! besides what we discussed together (needing to copy amounts over to puvspr_calculations for features from puvspr and from legacy, but this is a separate concern, not for this PR), please see my notes inline.

In practice, once you've dealt with the inline notes (which could mean do nothing, or just removing the CORS reflection config, up to you), merging this will be ok from my point of view.

But, importantly, if we keep the current setup that changes how feature_list is shaped in the response tiles, we need to coordinate with FE for them to parse the new format of these tiles correctly.

api/apps/geoprocessing/src/main.ts Outdated Show resolved Hide resolved
Comment on lines +30 to +31
pu.id AS scenario_pu_id,
ARRAY_AGG(DISTINCT concat_ws(':',species.feature_id::text,pu.amount)) AS feature_list
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure we want to let go of the "plain" feature_list (without amounts)?

Thinking about use cases such as showing "how many features are present, irrespective of their amount, in any given PU", I'd lean on keeping that.

For the specific use case of counts of features per PU in practice that could be even compressed to a plain count, not even to an array of the UUIDs of the features present in the PU, and this could be easily done at query time, or through a generated column that holds the count of items in feature_list.

tl;dr should we insert the data above into a new column such as feature_and_amount_list (this being an array of [uuid, float] pairs, as a new CREATE TYPE feature_amounts AS (feature_id UUID, amount FLOAT)), and then keep storing the plain list of feature uuids into the existing feature_list column, and then if we want/need, add a new column such as feature_count int generated always as (cardinality(feature_list)) stored?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Everything related the api Ready to review PR ready to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants