Thanks @akeeste for suggesting these changes and I can confirm that WECSim will run with a fixed-step solver when simu.dt is not equal to simu.dtOut; however, what is strange is that when running with simu.dtOut is double simu.dt in our RM3 model we get different results than when both variables are the same. The first image below shows the base example case with the same sampling rate (for both variables) while the lower image is when simu.dtOut is double simu.dt where we pick up a low frequency oscillation in the output. I double checked the WEC-Sim master branch and when using ode45 rather than ode4 this behavior is not reproduced. The ode45 implementation provides the same output time histories. So this somehow still sees to be an issue only for ode4 with different sampling rates between simulation and output.

This is strange as you have only changed a bus to a mux and the To Workspace block should not have an effect on the actual system dynamics since this is just logging data.

@nathanmtom This is very strange! I reverted back to master and redid my fix to ensure nothing else changed, but this oscillation still occurs. I'll keep investigating and see if another fix does not cause this issue.

@dforbush2 and @akeeste what's the status on this PR?

@kmruehl Still not clear why this happens...checking if it is something related to MATLAB version, since this has not been noticed till recently.

To update: Error replicated in 2020a (oldest library version), 2020b, and 2021b.

Error does not occur with all fixed-step solvers: using ODE1 shows the expected results, while ode2 and ode4 show the erroneous low-frequency component. This suggests that it is a Simulink solver implementation issue and is not a product of our code.

Changing the output generation at the top level as per https://www.mathworks.com/help/simulink/gui/output-times.html is NOT a robust solution because this parameter is only read for variable-step solvers and, to my investigation, cannot easily be made active for fixed-step solvers.

To pursue a solution, @akeeste has contacted MathWorks. In the short term, it is recommended that any re-sampling of output data takes place in post-processing.

I have received a response from Mathworks support on this issue:

"In short, we believe this is a bug and the root cause is due to the multitasking executed in normal making the "if-else" and "merge" block cannot correctly execute during the minor steps. 

Please refer to the attached "pic1" for the problematic "if-else" block. 

As for a workaround, there are several ways:
1. Keep the "if-else" block and disable the multitasking (RM3 Simulink -> Model Configuration -> Solver -> Solver details -> Uncheck treat each discrete rate as a separate task)

2. Replace the "If-else" & "Merge" with "Switch" as shown in "pic2". 

3. Keep the "if-else" block, simulate the model in Accelerator mode. 

I believe this solves your issue and hope the workaround works for you. I have submitted a bug report on behalf of you and our development team will consider fixing it in the future release.
"

I only see two If blocks in our model:
WECSim_Lib_Body_Elements/Rigid Body/Hydrodynamic Body/Wave Diffraction and Excitation Force Calculation/If2
WECSim_Lib_Body_Elements/Flex Body/Wave Diffraction and Excitation Force Calculation/Wave Diffraction and Excitation Force Calculation/If2

In response to the proposed solutions, # 1 seems like it might effect wecSimPCT though I not checked, # 2 is possible, # 3 is undesirable as we don't want to confine users to Accelerator mode.

As a 4th option, we can move the if statement and ramp to a *.m function. I confirmed that this successfully works around the solver bug. The ramp is a simple enough function, but it is defined in many different places in WEC-Sim. It would be better to use a single function for all of these ramp calls, which is easiest to implement with a *.m function.

Adam

I updated this branch to

1. change the bus to mux for body To Workspace blocks (1 in rigid body, 1 in nonhydro, 1 in drag, 2 in flex)
2. change the bus to mux for mooring To Workspace blocks (1 in mooring, 1 in MoorDyn)
3. change the bus to mux for PTO-Sim To Workspace blocks (11 total, 1 in each block)
4. Replace the If blocks in the rigid and flex body excitation forces to use a switch instead

@akeeste and @nathanmtom is this ready to be merged into master?

@kmruehl @nathanmtom @dforbush2 This should be reviewed one more time since I implemented new changes to the library.

@akeeste @kmruehl I've just reviewed these changes and can confirm that the low frequency oscillation is no longer present when running ode4 when simu.dt and simu.dtOut are not equal. Therefore, this bug fix should address the initial issue with WEC-Sim erroring out when have the simulation and output time steps at different rates and the later issue of a low frequency signal in the output time history.

I do want to acknowledge @akeeste comment about transitioning the ramp function to a *.m file. In the initial development of WEC-Sim, if I remember correctly, where trying to do all computations with Simulink blocks as at the time it proved computationally faster. Given our recent conversion from Simulink blocks to *.m files this seems like a reasonable script to make as it is applied for all instants where we are calculating the wave elevation or wave-excitation forces. This is not needed for this PR, but do not want to lose discussion on that topic.

However, these does seem to be a conflict with the mooring library that needs to be resolved before merge.

@nathanmtom In a different branch I have been working on combining some duplicate functions. I think I can finish implementing a ramp *.m there, but that will be after v5.0

and thanks for pointing out that conflict. The other PR causing this was just merged. I will pull those changes in next week so that we can merge this.

@akeeste LMK when you've updated the Mooring library block and this is ready for a merge

@akeeste LMK when you've updated the Mooring library block and this is ready for a merge

@kmruehl The mooring library is merged and this PR is ready