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

resolve sankey caching bug #1180

Merged
merged 4 commits into from
Jan 9, 2024

Conversation

marc-vdm
Copy link
Member

@marc-vdm marc-vdm commented Dec 21, 2023

Sankey caching had 2 bugs:

  1. Reference flows with same activity name would fall under same cache item, despite potentially being different activities (e.g. different product/location/database).
  2. Caching did not cache static lca object, meaning it would keep updating when other items were selected (and new graph traversal calculation was started). While the graph was static, items like lca score, reference flow name and unit were updated to the last calculated sankey.
    • This data is now cached as metadata.
    • LCA object is now dropped from the cache as it serves no purpose but did increase size of the cache

Also; some log data was moved to debug, function call was re-ordered and explanation of items in the cache_key was added.

Checklist

  • Keep pull requests small so they can be easily reviewed.
  • Update the documentation, please follow the numpy style guide.
  • Update tests.
  • Categorize the PR by setting a good title and adding one of the labels:
    bug, feature, ui, change, documentation, breaking, ci
    as they show up in the changelog.
  • Link this PR to related issues by using closing keywords.
  • Request a review from another developer.

@marc-vdm marc-vdm added the bug Issues/PRs related to bugs label Dec 21, 2023
@marc-vdm marc-vdm added this to the 2.9.4 milestone Dec 21, 2023
@marc-vdm marc-vdm requested a review from mrvisscher December 21, 2023 16:52
@coveralls
Copy link

coveralls commented Dec 21, 2023

Coverage Status

coverage: 50.561% (-0.07%) from 50.634%
when pulling 67058d4 on marc-vdm:sankey_caching_bug
into f22833f on LCA-ActivityBrowser:master.

@marc-vdm marc-vdm marked this pull request as draft January 4, 2024 14:37
@marc-vdm marc-vdm marked this pull request as ready for review January 4, 2024 14:43
@mrvisscher mrvisscher merged commit f41d8d2 into LCA-ActivityBrowser:master Jan 9, 2024
12 checks passed
Copy link

github-actions bot commented Jan 9, 2024

Note

This issue has been implemented in the new release of Activity Browser 🚀 (version 2.9.4), you can get the new version by updating Activity Browser.

Do you want to be notified of new releases of Activity Browser? Subscribe to our updates mailing list ✉.

🤖_beep boop! I'm a bot and this message was an automated action._
If updating does not make sense for this issue, just ignore this.

marc-vdm added a commit to marc-vdm/activity-browser that referenced this pull request Jan 11, 2024
@marc-vdm marc-vdm deleted the sankey_caching_bug branch May 14, 2024 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues/PRs related to bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sankey caching bug
3 participants