-
Notifications
You must be signed in to change notification settings - Fork 57
Fix failing tests on dev #95
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
Conversation
|
@jtgrasb the latest commit should fix the OWC test |
|
@jtgrasb On the MOST application -- The current case is quite outdated now. There are some input file changes (easy to make) and some MOST pre-processing changes (hard for me to make). There are many changed variables on the pre-processing side. I suggest we remove the old case and just replace it with one of the up to date OSS options here: https://github.com/MOREnergylab/MOST/tree/master/Examples |
|
@akeeste Thanks for looking into it. That sounds like a good solution. Do you want to handle those updates either adding it onto this PR or creating a new PR? |
|
I'll add it onto this PR, hopefully it will be quick |
|
Hey @jtgrasb re the passive yaw bug--looking at the a series of WEC-Sim applications tests using the WEC-Sim dev branch: Runs #14, #15, #16, #17, #18. All tests use the same WEC-Sim_Applications commit: e8cacc3 and various points on the dev branch The passive yaw case passes on test run 17 (Dec 11, 2024, 9:13am CST) above and starts failing on test run 18 (Dec 11, 2024, 9:34am CST). Looking at commits on dev--this means WEC-Sim/WEC-Sim#1349 or WEC-Sim/WEC-Sim#1332 caused the change. 1349 exclusively changes the cable tension, so it would not be that. This leaves some change in the full directional spectra wave PR that may have caused a slight adjustment in the passive yaw regression |
|
The changes in #1332 come down to a directional binning fix and a change to spline interpolation from linear |
|
@jtgrasb these are the passive yaw regression results that I currently see: On dev and reverting 4 previous fixes/improvements (phase seed, PM formulation, wave direction binning, wave direction spline interpolation):
On dev and 3/4 changes (phase seed, wave binning, wave direction spline interpolation, but not PM formulation) I get an identical match to the original dataset. The current dataset must have been created after the PM formulation was corrected |
|
@jtgrasb we have successfully identified all of the sources of discrepancy between prior and current passive yaw simulations! I think we now have justification to replace the passive yaw "original" regression dataset with up to date output using the current dev branch I edited the previous comment to reflect updating the phase seed correctly. For future reference--credit to @jtgrasb for figuring this out--our current phase see definition on WEC-Sim dev is applied globally. Reverting back to the previous method requires restarting MATLAB. |
|
@jtgrasb the latest commit updates the passive yaw regression test data using simulation output using WEC-Sim dev |
|
Unsure why the OWC case failed last time--a MATLAB / runner / moorDyn crash for some reason. It passes on a rerun |
b5d1ee9 to
f9da1a0
Compare
|
The tests all pass locally, but the runner is inconsistent when using MoorDyn. I'm going to merge this PR to keep things moving and we can revisit certain applications as they come up. |
|
Thanks for getting this through the finish @akeeste! I will add it as a task to try to identify the source of the MoorDyn crashes. It's good at least we know that it is passing locally and can be rerun until it passes. |






There are a number of tests failing on the dev branch. This PR is to identify and resolve the issues with each of the failing tests:
I also added a workflow dispatch trigger to the tests on main and dev so we can easily rerun them.