-
Notifications
You must be signed in to change notification settings - Fork 149
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
Scenci #173
Conversation
Hi @danielhuppmann and/or @khaeru if you could please take a look, that would be great. To note: please do not squash and merge these should be two separate commits. The first enables the ability to pass options to the cplex solver (yes, we will want to refactor this eventually to be solver agnostic). The second adds an additional test to our scenario suite utilizing the barrier method for solving. @danielhuppmann you should be able to build off of this to test the pricing changes. Finally, i realize that I forgot to add release notes. Once this passes CI, I will do so, make the commits correct, and then we can merge it in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good point. I suspect it will still run (with defaults), but perhaps it is better to keep it as is. To note, the first time a user solves with the |
update readme for env vars update some scenci files add solve options to ci tests
Ok, this should now be good to go (after being reviewed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me, thanks @gidden
@danielhuppmann when this passes CI, I will "rebase and merge" so if you want to rebase #172 on this branch, you should be safe to do so |
ok @danielhuppmann all is good to go here |
Updating our integration tests to include multiple test cases and a scenario with a carbon budget