WDRT

[cmichelenstrofer]

This pull request is for transferring the rest of the WDRT functionality to MHKiT.

Capabilities:

• Buoy data I/O already in MHKiT
• Environmental contours are part of a separate PR
• [x] short-term extreme functions
• [x] sampling
• [x] extreme: full seastate
• [x] extreme: contour
• [x] extreme: MLER wave
• Equivalent fatigue load already in MHKiT

Examples:

• Contours example already in MHKiT.
• [x] short-term extreme example
• [x] Full sea state long-term extreme
• [x] Contour long term extreme
• [x] MLER long term extreme
• Fatigue will be done later

Tests:

• [x] Tests

Plan:

• Carlos: short-term extreme, sampling, full seastate, contour approach
• Chris: MLER

[cmichelenstrofer]

I've added the short-term extreme functionality and an example. I still need to add the function docstrings.

@hivanov-nrel can you review what I've done so far?

[cmichelenstrofer]

This PR would close #22

[cmichelenstrofer]

@hivanov-nrel what is the plan/timeline for MLER?

[cmichelenstrofer]

• [ ] Increase test coverage
• [ ] Pull in ssolson PR when it is ready

[hivanov-nrel]

@cmichelenstrofer are we pretty close on this? What is left to do?

[cmichelenstrofer]

STATUS OF WDRT MIGRATION TO MHKiT

@cmichelenstrofer latest changes (@hivanov-nrel can you review?):

• Created a (non-hidden) waves.contour module, rather than importing the functions into waves.resource
• Added 2 sampling functions for the full sea-state and contour approaches (waves.contours)
• Added a full seastate extreme function to loads.extreme
• Created examples for the full seastate and contour approaches. Modified the MLER example name and intro so that these three examples are consistent (three different methods to do same thing)
• Merged the changes from the main repo including @ssolson's contours methods

@cmichelenstrofer older changes (already reviewed by @hivanov-nrel )

• Create loads.extreme module and add short-term extreme methods
• Short-term extreme example

@hivanov-nrel changes (see @cmichelenstrofer review to follow):

• add loads.MLER module with MLER functionality
• add MLER example
• add MLER tests

@ssolson (separate repo)

• Migrate all the different contour methods, create a single function which can call on any specified method
• Have the contour method output all relevant information (needed for sampling)
• Create contour example showing the different methods
• Add contour tests

TO DO (CURRENT RELEASE)

• [x] Tests: short-term extreme methods
• [x] Test: sampling: full seastate
• [x] Test: sampling: contour approach
• [x] Test: long-term extreme
• [x] @hivanov-nrel review latest changes
• [x] @hivanov-nrel finalize MLER based on review
• [x] @hivanov-nrel can you verify that the examples run (I do not have the ability to pull the buoy data using MHKiT, I had to manually download it).
• [ ] @rpauly18 @ssolson final review

TO DO (FUTURE)

• [ ] Add a fatigue load example similar to the one in WDRT. MHKiT has an equivalent load function similar to the one in WDRT, so maybe there is no need to migrate WDRT's function. Someone should verify this. Even then, the WDRT example calculates a long term fatigue load, not just based on the observation time period. I.e. it converts from the 1 hour to a 1 year fatigue load. This capability should be migrated to MHKiT and showed in an example.
• [ ] Migrate missing functionalities from WDRT Contours (can be done at a later date, I added place holder functions for these in waves.contours. This include calculating steepness curve and modifying contours appropriately, adding confidence intervals on the calculated contours using bootstrap, etc. These would need tests and examples too (e.g. the existing environmental contours example).
• [ ] Improve explanation of MLER in the example (what is the "desired" response Md?)

[cmichelenstrofer]

@hivanov-nrel @ssolson @rpauly18 Unfortunately I won't be able to spend much more time on this. I provided a detailed summary (see previous comment) of the current status and what is left to do. I think we are most of the way there and should try to finish it soon (this coming release) rather than waiting for the next release. I will work with @hivanov-nrel on finishing this.

@ssolson the plan was for Adam to replace me in the coding efforts starting Q2, is he up to speed? Maybe he can help with some of the future tasks.

For the missing/future functionalities: @hivanov-nrel can you do the Fatigue task?

@ssolson the missing contour capabilities (e.g. modifying based on steepness, adding confidence intervals) should be relatively easy and are related to the work you did in your branch. Can you or Adam take this one on?

[cmichelenstrofer]

@hivanov-nrel My MLER Review

Some comments/suggestions:

1. My preference would be not to include the readRAO function for a few reasons

   • The response does not need to be a rigid body DOF motion for a single body device, it can be anything (relative motion between rigid bodies, PTO extension, tension on a mooring line, etc.)
   • It should be up to the user to convert their RAO (from whatever source) to the required input arrays, we should not be doing interpolation, or converting from period to frequency (what if the user already has frequency instead of period in their file?)

2. The MLER function is not very large, maybe consider including it in the loads.extreme module where the functions for the other extreme loads approaches are.

3. In the MLER example

   • I would keep going a little further and create the wave time-series, as this is the actual input (as boundary conditions) to the high fidelity (e.g. URANS) code
   • I would explain a bit more how this would then be used in a high fidelity code and what the design/extreme response is.
   • you say "we would like to find the wave profile that will generate a heave response of 1 meter for our WEC device". I am confused, how do you know the heave response beforehand? Isn't that the whole point of this example, to find the extreme heave response? I think you need to add more explanation on how the MLER method works, what the goal is, and how to use the results.

[cmichelenstrofer]

@hivanov-nrel The broken tests are fixed. Can you check if this latest push fixes the full sea state example for you? I will get started with the new tests.

[hivanov-nrel]

Yup it looks like it all works! Okay I'm working on MLER and then short term extreme tests if you arent there already.

[cmichelenstrofer]

I haven't gotten started on the new tests. Will update here as I complete them. I will mostly work on this in the afternoon, got a few morning meetings.

[cmichelenstrofer]

@hivanov-nrel I think the MLER test is failing

[hivanov-nrel]

@rpauly18 @ssolson hi all, Carlos and I were able to finish integrating the WDRT functionalities. Issues were created for future upgrades for whatever we couldnt get to. Sorry for the last minute request, but if you can review the changes we might be able to squeeze this into the next release!

[cmichelenstrofer]

Closes #22

[cmichelenstrofer]

@ssolson That makes sense. The short-term extreme example is the most "technical one". I think the full sea-state and contour examples are more approachable (but probably can still be improved in the ways you pointed out).

[cmichelenstrofer]

@hivanov-nrel Can we setup a time this or next week to address the review comments (assert statements, example notebooks, etc.). @rpauly18 and @ssolson is there anyone else that can help with these cleanup/style changes? I know @hivanov-nrel and I have very limited time.

[ssolson]

Realistically it is going to take a new person 2 or 3 times as long to learn the code, make the changes, and then get the changes reviewed by the original authors compared to the original authors making the changes. If you guys have commitments that prevent you from completing the PR then I can do it but I would not recommend including this PR in a tagged release if we go this way.

[hivanov-nrel]

I can carve out some time to finish this. Please let us know once the review is complete and Carlos and I can meet to discuss. If we wanted to get this in before the next release, what is the deadline we are aiming for?

[ssolson]

I would encourage you guys to address general comments around style, spelling, example descriptions, and code cleanup as soon as possible. My understanding is that this PR needs to be wrapped up to tag the release along with Dolfyn. I believe the release should have already gone out.

[hivanov-nrel]

@ssolson it looks like Github actions isnt automatically running the tests. Do you know why that would be? It looks like the workflow file wasnt changed and looks like it is set up similar to all the other PRs.

[cmichelenstrofer]

@hivanov-nrel I think I know what the issue is. We switched to a PR to the dev branch instead of main. @rpauly18 and @ssolson if the dev branch is now the default the GitHub actions need to be updated to run on PR to dev instead of PR to main.

[rpauly18]

@hivanov-nrel I think I know what the issue is. We switched to a PR to the dev branch instead of main. @rpauly18 and @ssolson if the dev branch is now the default the GitHub actions need to be updated to run on PR to dev instead of PR to main.

Good catch. We should set it to run on both PR to dev and master.

[cmichelenstrofer]

@hivanov-nrel can you check if the extreme_response_full_seastate_example.ipynb and the extreme_response_contour_example.ipynb run? I don't have the ability to run these because of the buoy data request

[cmichelenstrofer]

@ssolson the code is ready for you to continue reviewing. The only thing left is making the examples easier to follow. I will get to that at some point this week. @hivanov-nrel can you take care of the MLER example, I will take care of the other 3.

[cmichelenstrofer]

@hivanov-nrel I have cleaned up the contour and full sea state long-term extremes examples. The only examples left to clean are the MLER and the short-term extreme. I will work on the short-term extreme later.

[hivanov-nrel]

@hivanov-nrel I have cleaned up the contour and full sea state long-term extremes examples. The only examples left to clean are the MLER and the short-term extreme. I will work on the short-term extreme later.

@cmichelenstrofer i looked over the MLER example and the coding part is pretty straightforward. I have reached out to Eliot for better clarification regarding the theory, but no response yet. Ill try to follow up, but i think that shouldnt stop us from adding it to the release.

[cmichelenstrofer]

@ssolson or @hivanov-nrel can one of you re-run (clear outputs, restart kernel, run all, save, git add/commit/push) the following two examples:

• extreme_response_full_sea_state_example.ipynb
• extreme_response_contour_example.ipynb

I still haven't been able to figure out how to run those from my computer... I want to make sure they run and that the saved outputs match the new changes.

[cmichelenstrofer]

@ssolson I have finished cleaning up the extreme.py file and the last example short_term_extremes_example.ipynb. I will wait for the rest of your review. Thanks!

[ssolson]

Carlos the full sea state example is failing for me. The other 3 examples all run.

[cmichelenstrofer]

@ssolson the example should work now. Thanks!

[cmichelenstrofer]

Thanks @ssolson! But no one addressed my comment above. @ssolson @hivanov-nrel can one of you re-run those two test cases and push?

[cmichelenstrofer]

Also, I only switched to a PR on the main branch so that the tests would run, this should have been merged into the dev branch. I think it's fine this time.

[ssolson]

@cmichelenstrofer I re ran the test prior to merging and they worked.

I did not notice this was switched but ultimately I believe this is the correct way. I really dislike the dev branch. There are tagged releases on github and pip installs. I don't think the only way the Matlab version can reach the tagged release is via us having another branch on the repo. There is a better way to do this than making us maintain multiple branches on the main repo. I was hoping to tag the release today anyway which would move it into master. Just waiting to hear from James about taging the main Dolfyn dependency.

[cmichelenstrofer]

@ssolson I wanted someone to push the examples too, in addition to checking that they run. So that the saved outputs match the new changes.

[hivanov-nrel]

@ssolson @cmichelenstrofer im running them now and will push the updates directly to the master branch

[hivanov-nrel]

Agh.....it looks like i dont have permission to push directly to master.... @ssolson do you want me to create another PR for this that you can merge? Or do you just want to run them yourselves and commit to master?

[ssolson]

@hivanov-nrel could you please submit a PR?

[hivanov-nrel]

@ssolson, sorry for the delay, i had another appointment. The PR has been submitted, please merge when you can. Thanks.