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

GTC-2889 Remove code in FCD related to geometry diffs and intermediate results #241

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

danscales
Copy link
Collaborator

GTC-2889 Remove code in FCD related to geometry diffs and intermediate results

We've been ignoring geometry diffs (location id == -2) for a long time, and we never pass in any intermediate results.

By removing this code, we avoid computing the centroid of each location geometry, inserting it into the feature id, then removing it from the feature id without having used it at all.

Move the only remaining code from combineGridResults() into the main function. Also, we had a duplicate call to
data.withUpdatedCommodityRisk() in combineGridResults(), so got rid of that (already called data.withUpdatedCommodityRisk() in main analysis code above).

The test change was just a change in order of categories within a single result, which is no actual change of the results.

…e results

We've been ignoring geometry diffs (location id == -2) for a long time,
and we never pass in any intermediate results.

By removing this code, we avoid computing the centroid of each location
geometry, inserting it into the feature id, then removing it from the
feature id without having used it at all.

Move the only remaining code from combineGridResults() into the main
function. Also, we had a duplicate call to
data.withUpdatedCommodityRisk() in combineGridResults(), so got rid of
that (already called data.withUpdatedCommodityRisk() in main analysis
code above).

The test change was just a change in order of categories within a single
result, which is no actual change of the results.
@danscales danscales requested a review from jterry64 July 11, 2024 18:30
Copy link
Member

@jterry64 jterry64 left a comment

Choose a reason for hiding this comment

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

This simplifies the analysis a lot! Looks good to me.

@danscales danscales merged commit bfd49b6 into master Jul 12, 2024
3 checks passed
@danscales danscales deleted the remove-diff branch July 12, 2024 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants