Added Floating Platform Model and tests #128
Conversation
hopp/offshore/floating_platform.py
Outdated
| return {} | ||
|
|
||
| # Define individual calculations and functions to use outside or with ORBIT | ||
| def calc_substructure_mass_and_cost(mass, area, depth, fab_cost=14500., design_cost=4.5e6, sub_cost=3000, pile_cost=0): |
There was a problem hiding this comment.
fab_costandsub_costseem to be used as rates. The names should reflect that.- please add doc strings to the functions specifying what each input is, where the default values come from, what the units are, etc
- pile cost does not appear to be used. Is there any reason to include it in the call signature?
| #phase = "H2 Floating Platform Design" | ||
|
|
||
| # Expected inputs from config yaml file | ||
| expected_config = { |
There was a problem hiding this comment.
please include units for all attributes of the input config
| #phase = "H2 Floating Platform Installation" | ||
|
|
||
| # Expected inputs from config yaml file | ||
| expected_config = { |
There was a problem hiding this comment.
please include units in the description for each attribute of the config
There was a problem hiding this comment.
I appreciate the unit inclusion. However, I am concerned that the config files do not conform to standard files in ORBIT. Can you adjust the config files to match ORBIT where they came from but leave the units in names in the code itself? The config files should still have comments specifying units.
hopp/offshore/floating_platform.py
Outdated
| '''Topside Cost & Mass | ||
| Topside Mass is the required Mass the platform will hold | ||
| Topside Cost is a function of topside mass, fab cost and design cost''' | ||
| topside_cost = topside_mass *topside_fab_cost_rate +topside_design_cost |
There was a problem hiding this comment.
This is a bit nit-picky, but it is helpful to have no spaces around multiplication and have spaces on both sides of addition and subtraction. This approach helps to group things visually the way they will be operated on. See https://peps.python.org/pep-0008/#whitespace-in-expressions-and-statements for more information.
There was a problem hiding this comment.
If you want to do this automatically, there is a tool called "black" that auto formats code according to PEP 8 standards. Probably does not make sense to include it in HOPP yet, but I'd like to eventually.
| return platform_capex, platform_mass | ||
|
|
||
| #@process | ||
| def install_platform(mass, area, distance, install_duration=14, vessel=None): |
There was a problem hiding this comment.
are the mass and area here just the platform mass and area or do they include the equipment?
| opex_rate = 0.01 | ||
| cost = calc_platform_opex(capex, opex_rate) | ||
|
|
||
| assert cost == 28e4 |
There was a problem hiding this comment.
Can you add tests for the classes?
| assert pytest.approx(cost) == 8142358.32 | ||
| assert pytest.approx(mass, 1.) == 280.0 | ||
|
|
||
| def test_calc_platform_opex(): |
There was a problem hiding this comment.
Where did the test values come from here? Please specify where the testing values came from or how they were determined. Provide references if relevant
There was a problem hiding this comment.
Most things look fine now, but I still don't see any indication of where the test values came from. Please add some comments to the tests on the where the values came from.
…Add example yaml file for floating
…Add example yaml file for floating
|
Above concerns addressed in newest git push except for tests for classes, will look into that |
hopp/offshore/floating_platform.py
Outdated
| '''These arrays were pulled from Rebecca Fuchs SemitTaut_mooring cost models''' | ||
| depths = np.array([0, 500, 750, 1000, 1250, 1500]) # [m] | ||
|
|
||
| anchor_costs = np.array([28823, 112766, 125511, 148703, 204988, 246655]) # [USD] |
There was a problem hiding this comment.
Having these copied over and hard coded makes me nervous. They don't appear to have a year associated with them or a source and will certainly change with time. Does ORBIT give a source for these? Is it possible to use ORBIT more directly so we don't have so much copy-paste action going on and can stay update more easily?
hopp/offshore/floating_platform.py
Outdated
| finterp_anchor_cost = interp1d(depths, anchor_costs) | ||
| anchor_cost = finterp_anchor_cost(depth) | ||
|
|
||
| total_line_costs = np.array([430315, 826598, 1221471, 1682208, 2380035, 3229700]) # [USD] |
There was a problem hiding this comment.
see previous comment
There was a problem hiding this comment.
Where did these test values come from?
Also, it is better to only have on assert per test method or function
| #phase = "H2 Floating Platform Installation" | ||
|
|
||
| # Expected inputs from config yaml file | ||
| expected_config = { |
There was a problem hiding this comment.
I appreciate the unit inclusion. However, I am concerned that the config files do not conform to standard files in ORBIT. Can you adjust the config files to match ORBIT where they came from but leave the units in names in the code itself? The config files should still have comments specifying units.
… variable names to match Orbit Nomenclature in yamls in .py
|
Updated to reflect above concerns and comments |
| assert pytest.approx(cost) == 8142358.32 | ||
| assert pytest.approx(mass, 1.) == 280.0 | ||
|
|
||
| def test_calc_platform_opex(): |
There was a problem hiding this comment.
Most things look fine now, but I still don't see any indication of where the test values came from. Please add some comments to the tests on the where the values came from.
…cal report and update to test file
| @@ -5,6 +5,10 @@ | |||
| from ORBIT import load_config | |||
| from hopp.offshore.fixed_platform import install_platform, calc_platform_opex, calc_substructure_mass_and_cost | |||
|
|
|||
| '''Sources: | |||
| - [1] 2017 ORBIT Technical Report: https://www.nrel.gov/docs/fy17osti/66874.pdf | |||
There was a problem hiding this comment.
Can you just add in the authors, title, etc for the citation? Otherwise I think we are ready to merge.
Created Floating platform capex, opex and install cost modeling from ORBIT and Rebecca Fuchs Repo for the mooring and anchors. Accompanied with a test file.