Skip to content
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

Adding load mitigating controls and OWC #39

Merged
merged 17 commits into from
Nov 14, 2023
Merged

Adding load mitigating controls and OWC #39

merged 17 commits into from
Nov 14, 2023

Conversation

dforbush2
Copy link
Contributor

@dforbush2 dforbush2 commented Aug 8, 2023

This is an updated applications case from https://marineenergyjournal.org/imej/article/view/68 updated to WEC-Sim v5.0. It works. Documentation is included.

Also included the OWC orifice example from OREC training reworked slightly for applications case

@kmruehl kmruehl requested a review from jleonqu August 9, 2023 14:59
@kmruehl kmruehl changed the title Adding load mitigating controls Adding load mitigating controls and OWC Sep 20, 2023
Copy link
Contributor

@jleonqu jleonqu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for developing this application case @dforbush2
I have reviewed the documentation and I have run the cases on my laptop with no issues. The documentation is well detailed and easy to follow. This PR is ready to merge.

@akeeste
Copy link
Contributor

akeeste commented Oct 17, 2023

Hey @dforbush2 A couple comments before merging:

OWC:

  • Is there a difference between OWC/OrificeModel/GBM1.slx and OWC/OrificeModel/GBM1_2022b.slx? Can we remove *_2022b.slx?
  • Is there hydroData for the OWC case?

Both OWC and Load Mitigating Control:

  • We'll need tests for both cases. I can add the test files and add them to the CI.
  • The LTI block in the OWC case requires the Control systems toolbox. Are there any other toolboxes required for OWC or Load_Mitigating_Controls? We should note that in the README's for users and test set-up.

@dforbush2
Copy link
Contributor Author

Hi @akeeste

  1. No. Just only one of those is built using the correct library version.
  2. This is a WAMIT test geometry test17a. I will add it in to the repo, I think it was foiled by the .gitignore.
  3. I'll have to add some test files.
    Good catch on the LTI...I can (and should) actually remove this dependency.

@akeeste
Copy link
Contributor

akeeste commented Oct 17, 2023

@dforbush2 There's a ton of other test fixes in #49. So as long as tests for these cases can pass, the rest should work once merged in dev.

@akeeste akeeste self-requested a review October 17, 2023 20:46
@kmruehl kmruehl assigned akeeste and unassigned jleonqu Nov 8, 2023
@dforbush2
Copy link
Contributor Author

Looks like I still need a test for the load mit controls...it also looks like the tests I wrote for the OWC don't actually run in the unit tests, so I could use some help here with the unit test code.

@akeeste
Copy link
Contributor

akeeste commented Nov 8, 2023

@dforbush2 I'll take a look at the test files and push/PR to your branch if possible.

@akeeste
Copy link
Contributor

akeeste commented Nov 9, 2023

@dforbush2 I made updates for these tests. See dforbush2#1. Once that PR is merged into your branch, we can merge this PR into WEC-Sim Apps

@akeeste
Copy link
Contributor

akeeste commented Nov 14, 2023

Merged dev into this branch to resolve conflicts. I will merge once the tests pass

@akeeste
Copy link
Contributor

akeeste commented Nov 14, 2023

@dforbush2 The Load Mitigating Control and OWC tests are passing, so i will merge this PR.

@kmruehl The MOST test is not passing, apparently because of some small numerical differences. There is an optimization routine for some of the pre-processing files, I'm guessing there is some variation in the optimization outcome, which is slightly changing the end results (on the order of 0.01%). A couple ideas: we could add the pre-processed files to the repo and test based on those inputs, and then afterwards test that the pre-processing steps work, or decrease the accuracy required by the tests

@akeeste akeeste merged commit 58029cb into WEC-Sim:dev Nov 14, 2023
22 of 23 checks passed
@akeeste akeeste mentioned this pull request Nov 29, 2023
6 tasks
@kmruehl
Copy link
Contributor

kmruehl commented Nov 30, 2023

Thanks @akeeste. Please look into solutions with the failing MOST tests because they are not failing on master and dev. Inlcuding @jtgrasb for awareness.

jtgrasb added a commit that referenced this pull request Dec 1, 2023
* Adding load mitigating controls and OWC (#39)

* Adding load mitigating controls

* adding OWC applications case example

* adding 2020b version

* adding hydroData source

* update readmes, linting

* adding OWC orifice data

* add h5 file, mod gitignore, add testOWC

* add LMC and OWC tests to CI

* add LMC and OWC tests

* remove R2020b simulink file, remove last LTI block

* remove extra library file

* fix some paths, toolboxes required by the tests

* fix issue from merging dev, remove paraview tests

* add OWC MCR case to tests

---------

Co-authored-by: akeeste <akeeste@sandia.gov>

* Common input files (#50)

* common RM3 hydroData and geometry

* common OSWEC hydroData and geometry files

* multiple wave headings

* update input files

* consolidate sphere hydrodata

* update input files

* revert nonlinear hydro files back to original

* fix tests

* remove outdated comments from input files

* Increase test tolerance (#51)

* Increase test tolerance

* Change to relative tolerances

* Increase relative tolerance for tower

* Update README.md (#52)

Co-authored-by: Kelley Ruehl <kmruehl@sandia.gov>

---------

Co-authored-by: dforbush2 <dforbus@sandia.gov>
Co-authored-by: akeeste <akeeste@sandia.gov>
Co-authored-by: Adam Keester <72414466+akeeste@users.noreply.github.com>
Co-authored-by: Kelley Ruehl <kmruehl@sandia.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants