Test capytaine lids #363
Merged
Test capytaine lids #363
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Make uniform shift default false and fix test_core * Allow for non-uniform shift for impedance
* fix tests * fix pioneer
* Pioneer tutorial realizations demo Add frequency array and realization study to beginning of pioneer tutorial * Update pioneer * Clean pioneer and update docs * remove print * Units * Update pioneer plots * Waves before frequencies * Explain less frequencies * Minor updates * Update pioneer * Merge branch 'dev' of https://github.com/sandialabs/WecOptTool into phases_demo
* bug bix : DC and Nyquist frequency should not be devided by two before ifft * Changed td_to_fd to scale single sided frequency components rather than TD signal * minor bug fix from issue332 sandialabs#332
* run CI on PRs against dev branch * revamped tutorial 1, including fix for sandialabs#293 * more tutorial cleanup and editorial changes * more cleanup and incorporated changes in sandialabs#315 * fixed tutorial 2 colormaps * finishing touches * reverted a few accidental changes * fixes as per Jeff's review comments --------- Co-authored-by: Ryan Coe <[email protected]>
* run CI on PRs against dev branch * coppied fundamental utility files * import utilities module * added utilities funtions to tut1 * added bem plot from utils * added bem plot from utils * updated sankey plot * updated check_radiation_damping * cleared outputs * corrected bug * changed Zi to hydro_impedance to be consistent with our variables name python convention * PR review edits * add grid to plots * removed draft functions in utilities.py * typo * Fixed one more typo I found while reviewing Daniel's changes --------- Co-authored-by: Ryan Coe <[email protected]> Co-authored-by: Michael Devin <[email protected]>
…abs#337) * post_processing docstrings - examples - parameters (order) * handle multiple phase realizations internally * Update wecopttool/core.py * making outputs lists * Update implementation to function with tutorial 1 for now * Update tutorials * Update LUPA * Make sure same WEC is passed in * Add test_utilities Revert "Add test_utilities" This reverts commit 27399f0. * Update utilities module * Update utilities * Update test_utilities * Update tutorial 1 utilities call --------- Co-authored-by: Carlos A. Michelén Ströfer <[email protected]> Co-authored-by: Carlos A. Michelén Ströfer <[email protected]> Co-authored-by: jtgrasb <[email protected]> Co-authored-by: jtgrasb <[email protected]>
* added initial file changes based on sphinx_multiversion docs and WEC-Sim implementation * removed sphinx-multiversion since it is no longer supported and made manual multiversion * now uses absolute paths, commented out linkcheck for debugging * fixed docstring errors in utilities module * updating files again that somehow got reverted * fixing path in conf.py * don't run tutorials (will revert later) * handle file moves correctly, fixed if statement to make other versions appear * fixed two bugs in versions template * reverted temp changes, changes latest to main * switched latest to main * main branch now in root directory of pages * fixed URLs with change from last commit * make other branches visible before building * switched main branch tag for more testing * fixed typo * switched dev branch to an existing branch * renamed main to latest, changed version.html file name to avoid confusion * added prints about moving files so Sphinx output isn't misleading * fixed typo with quotations * changed versions.html name back because that broke things I guess * modified contributing documentation to reflect changes * add logic to remove duplicate 'latest' branch * Fixed pathing when already on latest * remove typo * Troubleshooting complete, switching back to correct branches for deployment * Removed extra word in docstring * removed redundant function * fixed pathing so returns to same file (and fixes tutorial/API docs) * changed latest branch for demonstration * switched back latest branch for deployment
* removed conda environment from workflows since newer capytaine/wavespectra work with Windows * fixed unnecessary capitalization * still create CI conda environment to fix Mac environment failures * added conda env fully back in, push workflow deploys docs, split PR workflow * conda environment activates again * mambaforge instead of miniforge * manual cache reset * reset to older version of setup-miniconda to troubleshoot
This reverts commit f79ff8b.
…estCapytaineLids
Pull Request Test Coverage Report for Build 11580839722Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
I still want to do some testing of the computation time with the new mesh refinements and the lids, but other than that, the PR is ready to be reviewed. |
|
I tested the computation time of the BEM runs for each of the tutorials before and after adding the lids. Both the AquaHarmonics and LUPA tutorials have basically no change in computation time with the lid applied. The WaveBot and Pioneer both have significantly increased computation time, but this is due to the refined mesh (reduced mesh_size_factor from 0.5 to 0.2) to help eliminate frequency spikes. 
|
|
@dtgaebe will review |
dtgaebe
requested changes
Oct 29, 2024
dtgaebe
approved these changes
Oct 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR uses Capytaine’s new internal lid feature to remove irregular frequency spikes from the BEM results for each of the 4 tutorials. The irregular frequency spikes mostly occur in the added mass and radiation damping but may also impact the excitation forces. The internal lid does not completely remove irregular frequency spikes, but it moves them to higher frequencies (ideally above frequency range of interest). The mesh size and lid position can be tuned to move the spikes outside the frequency range of interest.
Type of PR
Checklist for PR
Additional details
WaveBot: The wavebot uses a frequency array containing 10 frequencies up to about 18 rad/s. The impact of irregular frequencies is small, but there is an impact to the result at about 5.6 rad/s where the added mass and radiation damping are lower without the internal lid. In tests of a more discretized frequency array, the irregular frequency spike was found to line up with about 5.6 rad/s, confirming the cause of this discrepancy. Note, I also needed to decrease the mesh size factor from 0.5 to 0.2 to move this frequency spike fully outside the range of interest.
Aquaharmonics: The AH tutorial uses a frequency array containing 10 frequencies up to about 8 rad/s. When testing a more discretized frequency array, there is a small spike within the array, but this does not show up in the 10 frequencies as shown below. Still, I applied the lid as it will be useful if users change the frequency array at all.
The LUPA example is unique because the float body has a hole in the middle (often referred to as a moonpool). But, the main issue lies in the fact that there is space between the spar and the float (i.e. the hole in the float is larger than the spar). We already have an option to fill the hole in to remove the frequency spike and this works okay. I think a better way to do it is to make the float hole smaller so that its flush with the spar, then apply a lid. I have shown the results for this below and implemented it in the LUPA tutorial.
Pioneer: Adding a lid to this tutorial along with reducing the mesh size factor from 0.5 to 0.3 was able to remove the frequency spikes as shown below. This led to a slight change in the power results (~2 W for regular waves).
