Current plan is to keep this PR all together here in WEC-Sim to be reviewed. After reviewing, the example cases will be moved to the applications repo, but first we need to review the controller class and library.

Hi @jtgrasb
I'm reviewing this. I feel like we ought to reconsider pushing this to an applications case with the amount of work that has been done with the integration into the library. Certainly need documentation to show the use and make toolbox requirements clear, but unless the additional toolbox requirements can actually break the rest of WEC-Sim for someone running with the minimum WEC-Sim The exception to this might be the MPC: as implemented, this seems potentially more appropriate as an example, but open to discussion on all points.

TODO Test: Someone with the bare minimum WEC-Sim required toolboxes test this and ensure functionality on everything except where toolboxes are missing (e.g., MPC).

Overall, this is excellent and is major addition to WEC-Sim functionality and I can see it serving a great role in future WEC-Sim courses especially if they are integrated with a broader Wave Energy course.

Detailed remarks to follow. I avoided making changes to the library/functions unless I was sure they were intended.

1. Is there an implementation difference between (1/z) and Memory blocks? My understanding was the latter were the more robust option for all of the different solvers a user might pick. Specifically, 1/z used in feedback loop for declutching, latching and maybe others?
2. Check Simulink function call for "latchingTime" in the Simulink model. It is empty but probably should call latchingTime.m.
3. MPC workflow is distinct and doesn't appear to have a corresponding library block (looks like maybe this was unintended: the MPC control block link was broken. If possible to re-link, please do so. The MPC overall stands out as more of an example than a library: doesn't seem to generalize to other WEC geometries, particularly those with coupled DOF? Also pretty sure this will not work as intended in a variable time step.
4. makePlantModel.m: could this leverage code already used in the state-space approximation? How does this method compare in terms of accuracy?
5. makePredictiveModel: This uses a c2d transform for discretization. I have a couple of concerns. 1) does this imply that MPC can only work for fixed step solvers? The ZOH method can cause some phase probably undesired phase behavior at high frequencies in its approximation of the continuous model.
6. S-function: the MPC uses an s-function for fExcPrediction. This has caused version compatibility issues in the past. Also, this requires the stats toolbox additionally with the 'regress' function.
7. See line 69 of fExcPrediction. Was this in-house code? Any licensing concerns?

Thanks for proposing this PR @jtgrasb, clearly there is a lot of work gone into getting this controller toolbox ready be be integrated into the WEC-Sim. My initial thought is that this is still better suited as a suite of WEC-Sim Application Cases as you already have the 5 controller cases for the RM3 (only float), but none of the other examples. Also the limitation for using the latching/declutching only in regular waves is a bit misleading since there are several papers out there that tie the hold/release times that are tied to the natural period of the device and this is going to perhaps result in quite a few questions from the user community which is going to have us dive deeper into control theory. By itself this would not be an issue, but considering where we draw the line for technical support I think needs to be considered.

Also, since the toolbox suite you have generated is not generalizable to say the OSWEC or a two body attenuator design because of needing to shift the hydro coefficients away from the center of gravity to a hinge I think prevents this from being used for a wider range of concepts that WEC-Sim can support. Given we have kept WECCCOMP separate and it has an advanced MPC controller available, I think this would make for an excellent applications case, but this is not ready to be pulled into WEC-Sim master. At least not at this time, I think research from both labs have shown that sysID using WEC-Sim is an option to generate a linearized system of equations that you can then use to build a controller. This would allow us to generalize the controller better to represent the wide diversity of WECs we see.

You are also using just the float of the RM3 model, when it was designed as a two-body system, and although it can still demonstrate the control approaches you have coded, in my opinion it will not be recognized by our users about the additional complexities such as including a second body or changing from say translational to rotational motion.

Again I think you have done a lot of great work to get the PR to be mergeable with WEC-Sim Master, but I still think it is still better suited as an application case.

More focused questions/comments are:

• controllerBlocks.png needs to be updated as two of the 5 controllers have "Force1" rather than "Force" as label. Also, if MPC is removed then that is not an option we will be making available but there still seems to be a lot of MPC functions available although a previous commit mentions removing MPC.
• I might be doing something wrong in the compiling, but when I build docs locally and look for your updated section nothing is present. Potentially double check that with the latest push you did not do something to effect the docs build. There also seems to be continued mention of MPC but it sounds like you removed this.
• Similar, when I build the docs locally I am not seeing any of the new sections.
• Have you compared the MPC approach in our WECCCOMP applications case against what you are implementing? We may need to highlight the differences so user do not assume that they are implementing the same approach.

@dforbush2 Thanks for the comments!
Agreed, see Kelley’s comment, the plan is to have the examples reviewed here and then move them to WEC-Sim applications. One component to address is the controllerClass and library and whether those should be a part of WEC-Sim master or if all of it should be integrated into the examples for an applications case.

1. I’ll look into this. I didn’t use memory blocks simply because I’m not as familiar with them, but I’m guessing they can accomplish the same task, so I’ll update if possible.
2. Should be fixed, thanks!
3. This is intended - MPC functions require extra toolboxes so this was left out of the library so it could exclusively be an application.
4. Will look into
5. Will look into
6. I hadn’t realized the compatibility issues with the S-functions. Are there any past discussions that I could look at that reference this issue? As for the stats toolbox, this shouldn’t be an issue as it will be part of an applications case
7. This was code from the control-sim repo, which I believe was public at one point, so I think it should be okay? Will remove these comments from the code though

@nathanmtom Thanks for the comments!

1. Agreed on moving this to an applications case. That’s a good point about the latching and declutching - I can work on trying to implement latching/declutching for irregular if we think it's worth it, but tried to explain the issue in the documentation.
2. Agreed on moving this to an applications case.
3. Do you think I should simply replace the RM3 float with IES OEA Task 10 sphere? Or would you suggest more complex examples for multibody, other dofs, etc.? Maybe this could be a further addition to add to the project board?
4. Will look into and update Force vs. Force1. I apologize, this was a slight miscommunication with the commit - MPC is not removed, but was removed from the controllerClass.m and library to avoid issues with extra toolboxes in WEC-Sim Master.
5. Docs seem to be building fine on my end with the main updated section showing up in Advanced Features.
6. Good point, I will look more into the WECCOMP example to try to highlight the differences.

Great work on this PR @jtgrasb. The examples are clear and the documentation is well written. I made a couple of comments on the documentation one of them regarding the challenges of adjusting the natural frequency of the device to match the wave frequency. It would be useful to add a couple of comments on this.
I agree with Nathan, the examples are great but at this point I think is difficult to apply the library to a general case. As Nathan said, the examples that you have so far will be better suited for application cases, which will be really useful for WEC-Sim users.
Regarding multibody systems, I would suggest to have a different approach. Maybe we can formulate the equations with the control coefficients for a given controller, and then we can run an optimization routine to find the optimal coefficients for the multibody WEC. This is just an idea, but I think it could be useful for an application case.
Also, I was checking the simulink blocks, and all of then have the same description regardless of the type of block. All the blocks have the following description: Use WEC-Sim input file to define Latching Controller parameters. You should update all the blocks to have the appropriate description depending on the type of controller.

Thanks for all the comments and suggestions. I am working on incorporating your comments into a PR for the wec-sim applications case. I had some questions that I wanted to address first.

@dforbush2
For the state space approximation used by WEC-Sim, my understanding is that it is just used for the radiation calculation while the state space model for MPC needs to take into account excitation and radiation forces.
The c2d transformation here does restrict the MPC to a fixed step, but I think potentially the rest of the simulation could work with a variable step solver, but I’m not sure if it would work with the rate transition blocks. I think this is okay since it will be an applications case. As for the method (zoh, etc), I’m not too familiar with the different methods, is there a different one you’d suggest?
I’ve found a way to do it without S-functions, so working on transitioning to normal MATLAB functions now.

@nathanmtom
You mentioned sysID has been used with WEC-Sim before to create a linearized system of equations to build a controller - is there a paper you could direct me to on this topic?
For the comparison to the WECCCOMP example, I think the difference lies in the state space representation of the WEC motion and constraints (and obviously geometry), but I am still looking to pinpoint the exact differences in formulation.

@jleonqu
I’d like to talk more about the multibody approach you mentioned as well as trying to integrate these examples with PTO-Sim better to emphasize the importance of considering electrical power rather than mechanical power as it is now.

Best,
Jeff