-
Notifications
You must be signed in to change notification settings - Fork 187
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
Monte carlo Improvement #930
Conversation
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.
hello @GbotemiB :D
Great activity :) I've added some comments as discussed :)
Thanks @davide-f for the amazing review. |
Hello @davide-f, I have implemented all the requested changes. I think the PR is ready for another round of review. |
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.
Great revision @GbotemiB :D
Few minor comments :)
As a comments, in debug, may you show a plot of input and outputs provided by the rescaling function for different values?
Have you done that to make sure the expected distributions are verified?
@GbotemiB Emmanuel, please use precommit. To make sure you use the pre-commit, please check this that suggests you how to set it up |
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.
Partial revision for few comments :D
05c902d
to
7c490ad
Compare
685bdf3
to
f5b62c7
Compare
Hi @davide-f, I have successfully cleaned up the history. Thanks for the tip. Also, I have made changes to the part of the code you reviewed. Please feel free to review the changes. Thanks. |
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.
Nice @GbotemiB !
I was curious in validating the extraction of the coefficients by the monte-carlo.
The plots show the distribution of the run of selected montecarlos, but do not show the post-processed coefficients that modify the runs.
I think it should be good to verify that the coefficients that are extracted across the 59 simulations do have the shape we expect.
If you have the network basic elec network for example, you could re-obtain the coefficient for example doing:
coeff = sum(n_montecarlo.generators_t.p_max_pu[:, ... == solar ...]) / sum(n_elecnetwork.generators_t.p_max_pu[:, ... == solar ...])
the example above is for solar, but you could do the same for the other carriers and try get the curves.
I'd like to validate that what we expect being a gaussian is a gaussian for example.
I believe we can soon merge the PR :)
Hi @davide-f, I can confirm that the post-processed coefficients follow the pre-processed coefficients. These are some of the plots generated. I will include the plots in the notebook I am working on for the case study analysis and share the notebook with you. |
Hello @GbotemiB ,
Could you crosscheck? |
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.
Hello, closing to finalize, I added a comment on the shapes of the distribution that do not look like expected.
Could you clarify?
Regarding the distribution of the parameters, it follows this config. But I could reconfigure the parameters. uncertainties:
loads_t.p_set:
type: beta
args: [2, 0.5]
generators_t.p_max_pu.loc[:, n.generators.carrier == "solar"]:
type: beta
args: [4, 1]
generators_t.p_max_pu.loc[:, n.generators.carrier == "onwind"]:
type: beta
args: [4, 1]
generators.capital_cost.loc[n.generators.carrier == "solar"]:
type: normal
args: [0, 1]
generators.capital_cost.loc[n.generators.carrier == "onwind"]:
type: normal
args: [0, 1]
generators.weight.loc[n.generators.carrier == "onwind"]:
type: beta
args: [2, 0.5]
generators.weight.loc[n.generators.carrier == "solar"]:
type: beta
args: [2, 0.5] |
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.
Final rounds!
Great you added the reference to the api :) close to done! [I know it's the third time, but we are really there :)]
Happy to jump in if necessary.
Regarding the distribution plots, have you tested all distributions?
Co-authored-by: Davide Fioriti <[email protected]>
@davide-f, No problem about continuing the review if necessary. Feel free to jump in. |
@davide-f, Regarding the distribution, I have tested them. Here is a config for Monte-Carlo and the results. monte_carlo:
options:
add_to_snakefile: true
samples: 961 # number of optimizations. Note that number of samples when using scipy has to be the square of a prime number
sampling_strategy: "pydoe2" # "pydoe2", "chaospy", "scipy", packages that are supported
seed: 42 # set seedling for reproducibilty
uncertainties:
loads_t.p_set:
type: normal
args: [1, 100]
generators_t.p_max_pu.loc[:, n.generators.carrier == "solar"]:
type: uniform
args: [0.1, 1]
generators_t.p_max_pu.loc[:, n.generators.carrier == "onwind"]:
type: lognormal
args: [1.5]
generators.capital_cost.loc[n.generators.carrier == "solar"]:
type: triangle
args: [0.7]
generators.capital_cost.loc[n.generators.carrier == "onwind"]:
type: beta
args: [0.5, 1]
generators.weight.loc[n.generators.carrier == "onwind"]:
type: beta
args: [2, 0.5]
generators.weight.loc[n.generators.carrier == "solar"]:
type: gamma
args: [2, 0.5] |
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.
Great @GbotemiB :D
The PR is ready to merge, really great great job :D
Closes # (if applicable).
Changes proposed in this Pull Request
This PR includes improvement on the Monte-Carlo simulation. The improvement includes the option to choose from different distributions for the simulation. The distribution includes normal, triangle, lognormal, beta, and gamma. While choosing the distribution, it also allows users to pass parameters for the distribution.
Checklist
envs/environment.yaml
anddoc/requirements.txt
.config.default.yaml
andconfig.tutorial.yaml
.test/
(note tests are changing the config.tutorial.yaml)doc/configtables/*.csv
and line references are adjusted indoc/configuration.rst
anddoc/tutorial.rst
.doc/release_notes.rst
is amended in the format of previous release notes, including reference to the requested PR.