-
Notifications
You must be signed in to change notification settings - Fork 20
92 simplify and update offshore cost model #169
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
base: dev
Are you sure you want to change the base?
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.
I added some comments @mstargardt
Also we need the pipeline running for RESKit on Github before we can merge anything @julian-belina
# Combine all costs | ||
totalOffshoreCapex = ( | ||
newTurbineCost + newFoundationCost + newCableCost + overheadCostBase | ||
) | ||
|
||
return totalOffshoreCapex |
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.
also this function would profit from having the respective unit next to the variables
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.
The pipeline is up and running on the dev branch
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.
Very, very nice addition to the RESkit body, thanks @mstargardt !! Only one general question left, also to @phil-fzj - would we discontinue the "offshore_turbine_capex" function and related functions in offshore_cost_model.py then? They are very nice, but helplessly outdated and cannot be updated without a new, consistent datasource as far as I understand @mstargardt ?
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.
some tests are failing, please ensure that the test pipeline works at least locally until the github version is adapted
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.
Substantial error in scaling, not the author's fault but probably wrong in source paper
I would propose ignoring the linting error and introducing linting in a new pull request. |
closes #156 |
added new commits. last test run needs to be done. |
Added function to scale given OffshoreCapex regarding, wind speed, water depth and distance to the nearest coastline.
Test are included