Skip to content

Comments

Merge PACs stuff with flows for processes#352

Merged
alexdewar merged 3 commits intomainfrom
combine-pacs-flows
Jan 23, 2025
Merged

Merge PACs stuff with flows for processes#352
alexdewar merged 3 commits intomainfrom
combine-pacs-flows

Conversation

@alexdewar
Copy link
Collaborator

Description

Every process has at least one commodity flow, at least one of which must be designated as a PAC (primary activity commodity). Currently this information is contained in two input files: process_flows.csv and process_pacs.csv. The latter just contains pairs of process and commodity IDs indicating which of the commodity flows are PACs for each process. The code reflects this input file structure: the Process struct has separate flows and pacs fields, which isn't particularly usable. It means if you want to find out whether a given commodity flow is a PAC you have to iterate over Process.pacs to check if it's in there. Conversely, to get the process flows corresponding to PACs, you have to find the flows whose commodity IDs match those in Process::pacs.

While the code doesn't actually use these data structures for anything other than validation yet, when we do come to use them (we need them for the dispatch optimisation), this will be annoying. We can simplify things by just adding a boolean flag to each process flow indicating whether or not it's a PAC. We can then delete the process_pacs.csv example file and most of the associated validation code, which is what I've done. I've moved the remaining PAC validation steps to src/input/process/flow.rs.

Unrelated change: I also tidied up some doc comments in src/process.rs.

Closes #337. Fixes #338.

Type of change

  • Bug fix (non-breaking change to fix an issue)
  • New feature (non-breaking change to add functionality)
  • Refactoring (non-breaking, non-functional change to improve maintainability)
  • Optimization (non-breaking change to speed up the code)
  • Breaking change (whatever its nature)
  • Documentation (improve or add documentation)

Key checklist

  • All tests pass: $ cargo test
  • The documentation builds and looks OK: $ cargo doc

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@codecov
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

Attention: Patch coverage is 96.37681% with 5 lines in your changes missing coverage. Please review.

Project coverage is 95.74%. Comparing base (a750026) to head (6df67e7).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/process.rs 0.00% 3 Missing ⚠️
src/input/process/flow.rs 98.51% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #352      +/-   ##
==========================================
+ Coverage   95.67%   95.74%   +0.07%     
==========================================
  Files          28       27       -1     
  Lines        3211     3129      -82     
  Branches     3211     3129      -82     
==========================================
- Hits         3072     2996      -76     
+ Misses         62       61       -1     
+ Partials       77       72       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tsmbland
Copy link
Collaborator

Cool, was about to start #248, so will build off this

Copy link
Collaborator

@tsmbland tsmbland left a comment

Choose a reason for hiding this comment

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

Looks good!

@alexdewar alexdewar merged commit 4361d9f into main Jan 23, 2025
7 checks passed
@alexdewar alexdewar deleted the combine-pacs-flows branch January 23, 2025 16:30
@alexdewar alexdewar mentioned this pull request Jan 23, 2025
10 tasks
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.

MUSE panics if processes are missing commodity flows Processes: Combine PACs and flows in code and CSV file structure

2 participants