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

New scenarios and bid config to match multi scenario 20 in v0, plus improved names, and adding 0.01 to buy_price #370

Conversation

eriklien
Copy link
Contributor

Description

  • New bid config equal to multi_scenario_20_NO2 in V0
  • More descriptive scenario (and mapping) names/ext_ids
  • Adding 0.01 to buy_price to stop Shop from selling and buying at the same time

Checklist:

@eriklien eriklien force-pushed the bid-configs-comparable-to-v0-plus-better-names-and-buy-price-plus-0.01 branch from 80216b6 to 5cef506 Compare July 24, 2024 12:44
@eriklien eriklien marked this pull request as ready for review July 24, 2024 12:47
@eriklien eriklien requested a review from a team as a code owner July 24, 2024 12:47
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: I think it'd be better if this notebook were consistent in creating a JSON object because right now, you have no2_price_scenario_mappings that are created as a JSON, but then in plants, you're printing each individual line out as a formatted string.

I also would suggest that this notebook directly dumps the JSON into the YAML file as opposed to printing out the YAML dump and copy-pasting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, but not sure I'll prioritise this now (and I think of the notebook as a hacky thing which is useful for avoiding some manual work, but maybe not the perfect way to set up these resources - maybe it should not be in the repo?)

Dumping to the yaml files should also be done with some caution in case there are some parts in the yaml file that should not be overwritten - then it would be good with multiple files for the same kind of config (such that the notebook or some other code could dump things into "its own" set of files).

Copy link

codecov bot commented Sep 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.89%. Comparing base (cf25c71) to head (2f5e68d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #370      +/-   ##
==========================================
- Coverage   49.00%   48.89%   -0.11%     
==========================================
  Files         566      566              
  Lines       40200    40286      +86     
==========================================
  Hits        19698    19698              
- Misses      20502    20588      +86     

see 86 files with indirect coverage changes

@eriklien eriklien merged commit 5ab3b57 into main Sep 12, 2024
7 checks passed
@eriklien eriklien deleted the bid-configs-comparable-to-v0-plus-better-names-and-buy-price-plus-0.01 branch September 12, 2024 09:12
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.

3 participants