-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix: extra functionality not called #28
Conversation
Validator ReportI am the Validator. Download all artifacts here. Ariadne Variables Comparison
NRMSE: Normalized (combined-min-max) Root Mean Square Error General Files comparison
NRMSE: Normalized (combined-min-max) Root Mean Square Error Model Metrics Comparing |
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.
In general it works and is looking good. It is also nice to include it with the infrastructure already provided by pypsa-eur.
However, what doesn't quite sit right with me is placing the custom_extra_functionality
script into the data folder. The data
folder in my opinion is for universal files that are used as input only.
The custom_extra_functionality
is changing the optimization problem profoundly so I would like to see it somewhere in ./scripts/
...
@lindnemi / @fneum what is your opinion on this? Is that something you'd like to see and if so only for pypsa-de
or even pypsa-eur
?
I couldn't agree more. We can just move the file and change the path in the default config for pypsa-de: pypsa-de/config/config.default.yaml Line 912 in c1e59d1
In Eur it is probably meant as a "data file" for users who do not touch any code and just add custom functionality with some changes in the config. It still doesn't make sense for this to be tracked by git. But this is a pypsa-eur problem. The data folder also needs a general clean up over there. For DE I would just move the file back to scripts and change the default config. |
2e0fc1e
to
a3ddba0
Compare
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Checklist
ariadne_all
completes without errorsmain
has been merged into the PR