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

Always attach efficiencies to flows #442

Merged

Conversation

gnawin
Copy link
Member

@gnawin gnawin commented Feb 6, 2024

Pull request details

Describe the changes made in this pull request

add_efficiency is used to create three sets for flows: lowest resolution without efficiencies, lowest resolution with efficiencies, and highest resolution without efficiencies.

  • Remove add_efficiency, so flows are always attached with efficiencies. At some cases, the efficiencies are set to 1. @datejada Would you double-check the conditions (by the variable flag) I added?

To still be able to have the set highest resolution without efficiencies

  • Add use_highest_resolution. We can open another issue if we want to get rid of this flag as well.

Also a minor edit

  • Add an empty line after log display

List of related issues or pull requests

Closes #436

Collaboration confirmation

As a contributor I confirm

  • I read and followed the instructions in README.dev.md
  • The documentation is up to date with the changes introduced in this Pull Request (or NA)
  • Tests are passing
  • Lint is passing

@gnawin gnawin requested a review from datejada February 6, 2024 20:37
Copy link
Member

@datejada datejada left a comment

Choose a reason for hiding this comment

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

I like the changes, thanks! It is more straightforward for the reader to see what is happening. I added some changes in variable names for clarity and consistency.

Copy link

codecov bot commented Feb 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (00a45de) 100.00% compared to head (7b285a8) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #442   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          528       526    -2     
=========================================
- Hits           528       526    -2     

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

@datejada datejada merged commit 1ed0a36 into TulipaEnergy:main Feb 7, 2024
7 checks passed
@gnawin gnawin deleted the 436-refactor-efficiencies-for-flows branch February 21, 2024 13:29
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.

Always attach efficiencies to inflows and outflows
2 participants