SNL-WaterPower / WecOptTool-MATLAB

WEC Design Optimization Toolbox
GNU General Public License v3.0
12 stars 9 forks source link

Minor plotting suggestion - example8Spectra from RM3 Example #186

Closed ThBoerner closed 3 years ago

ThBoerner commented 3 years ago

Hi Guys,

The documentation and walkthrough of the RM3 model is great! However, one might stumble over the paragraph which suggests plotting the example8Spectra which reads as: "In the active code above from "optimization.m", there are eight spectra loaded into a struct array. These can be plotted using standard MATLAB commands."

figure
hold on
grid on
arrayfun(@(x) plot(x.w,x.S,'DisplayName',x.note), S)
legend()
xlim([0,3])
xlabel('Freq. [rad/s]')
ylabel('Spect. density [m^2 rad/s]')

The example8Spectra as an object of the the seastate class does not own the "note" argument compared to the struct created from the WAFO toolbox. It might be nice to point that out or correct the plotting code snippet displayed in the tutorial.

I found the plot function included in the sea state class definition helpful and was wondering why it's not a static method to be accessed easily. It also nicely shows how the "trimFrequencies" method works and (for now) doesn't use the WAFO "note" argument.

As said, it's a minor suggestion and other than that, the examples run nicely!

Cheers Thomas

H0R5E commented 3 years ago

Hi @ThBoerner,

Thanks for pointing out the issue with the optimisation example. In a previous iteration of WecOptTool, all fields of the struct were recorded by the SeaState class, whether they were explicit attributes or not. In the current version, we have to set each attribute explicitly, so I've used PR #187 to record the note field if given in the input struct, and provide a default if it's not given.

I've also added a test to check the plotting code in the docs that wasn't working. Ideally we would be able to run doctests on code like that, but unfortunately doctests are not part of the MATLAB testing framework.

I found the plot function included in the sea state class definition helpful and was wondering why it's not a static method to be accessed easily. It also nicely shows how the "trimFrequencies" method works and (for now) doesn't use the WAFO "note" argument.

I'm not sure I understand how the plot method in SeaState would be useful as a standalone method, given its primary purpose is to illustrate the changes that the class has made to the original input spectrum. I'm going to close this issue with PR #187, but feel free to open another issue on this topic, perhaps including an example of how you might use that function in a static manner.

Thanks again,

Mat

H0R5E commented 3 years ago

This is what you get for rushing things. I had to finish this off with the direct commit 87c54b4 as the docs still referred to a struct array and used S rather than SS in the plot command. I think it's OK now.