Conversation
|
/build-installer |
|
Signed Windows installer built successfully. Download artifact. |
|
@daniel-rdt Worked correctly. See above. Sorry for adding noise to your PR, @measrainsey . |
daniel-rdt
left a comment
There was a problem hiding this comment.
Thank you @measrainsey for this PR. The settings look good to me, I only have minor comments on the location of the logs and PR description
|
|
||
| # Merge CBA-specific solving overrides into the global solving config | ||
| solving = snakemake.params.get("solving", {}) | ||
| solving = copy.deepcopy(snakemake.params.get("solving", {})) |
There was a problem hiding this comment.
I think one could drop the deepcopy as we are not reading the original snakemake.params.solving again but this is more robust, so let's keep it. Memory overhead is also negligable.
| PreDual: 0 | ||
| FeasibilityTol: 1.0e-05 | ||
| OptimalityTol: 1.0e-05 | ||
| ScaleFlag: 1 |
There was a problem hiding this comment.
Did you get to this scale attribute by testing? Probably good to document in the PR description what the value is based on
There was a problem hiding this comment.
Thanks @daniel-rdt for the suggestion!
I did a quick test for the ScaleFlag for the CBA MSV solve, and it looks like ScaleFlag: 0 is a bit faster, so for now I changed it to that (perhaps with more rigorous testing, a different attribute could be better).
- resolution: 1H
- snapshot start: 2019-01-01
- snapshot end: 2019-01-07
| Solver | ScaleFlag | 2030 MSV solve | 2040 MSV solve | Total |
|---|---|---|---|---|
| gurobi-default | 1.72 s | 2.10 s | 3.82 s | |
| gurobi-simplex | default (-1) | 1.79 s | 1.67 s | 3.46 s |
| gurobi-simplex | 0 | 1.65 s | 1.63 s | 3.28 s |
| gurobi-simplex | 1 | 1.72 s | 1.68 s | 3.40 s |
| gurobi-simplex | 2 | 1.73 s | 1.69 s | 3.42 s |
(also added to PR description)
Co-authored-by: Daniel Rüdt <117752024+daniel-rdt@users.noreply.github.com>
|
@daniel-rdt Thanks for your review! I accepted your suggestions about the logs locations (and updated the PR description to reflect the new location). For the |
daniel-rdt
left a comment
There was a problem hiding this comment.
Thank you @measrainsey for the changes and updating the PR description so thoroughly! I have one more thought on the ScaleFlag question. We can also have a brief chat about it later? might be quicker :)
| "FeasibilityTol": 1.0e-05, | ||
| "OptimalityTol": 1.0e-05, | ||
| "ScaleFlag": 1, | ||
| "ScaleFlag": 0, # 0 to turn off scaling (faster) |
There was a problem hiding this comment.
While your test have shown slight speed up with ScaleFlag 0, I am contemplating if our best option wouldn't be to let Gurobi decide automatically how to scale the model (i.e. ScaleFlag -1). It could be that the speed up that we are seeing with 0 here, is only because of the small size of the test model (1 week). The settings will be used for MSV on a whole year, will they not?
Closes:
Changes proposed in this Pull Request
PR to add option to use specific solver for CBA MSV, add
gurobi-simplexsolver option, and to move CBA-related loggers for easier debuggingTasks
gurobi-simplexsolver settinghighs-simplexas default solver for MSVresults/[run]/cba/logs/Workflow
Config files
gurobi-simplexas a solver option (not used as default anywhere)highs-simplexas default solver for MSV extractionLogs
logs/folder to a folder calledresults/logs/cba/, with separate subfolders formsv/,reference/,project/- for the different kinds of solves the CBA does.Gurobi simplex settings
For the
gurobi-simplexsolver option, the following settings are listed by default:ScaleFlag = 0was chosen based on a quick and small test that showed that it led to a slightly quicker solve than otherScaleFlagvalues.Open issues
Notes
Checklist
[ ] Changed dependencies are added topixi.toml(usingpixi add <dependency-name>).config/config.default.yaml.[ ] Changes in configuration options are documented indoc/configtables/*.csv.config/test/*.yaml.pixi run -e open-tyndp tyndp-cyears-test).[ ] Open-TYNDP SPDX license header added to all touched files.[ ] For new data sources or versions, these instructions have been followed.[ ] New rules are documented in the appropriatedoc/*.rstfiles.doc/release_notes.rstis added.[ ] Major features are documented with up-to-date information inREADMEanddoc/index.rst.[ ] Module docstrings added to new Python scripts.