Skip to content
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

Minor plotting suggestion - example8Spectra from RM3 Example #186

Closed
ThBoerner opened this issue Sep 28, 2020 · 2 comments · Fixed by #187
Closed

Minor plotting suggestion - example8Spectra from RM3 Example #186

ThBoerner opened this issue Sep 28, 2020 · 2 comments · Fixed by #187
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation

Comments

@ThBoerner
Copy link

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 H0R5E added bug Something isn't working documentation Improvements or additions to documentation labels Sep 29, 2020
@H0R5E H0R5E self-assigned this Sep 29, 2020
@H0R5E
Copy link
Member

H0R5E commented Sep 29, 2020

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
Copy link
Member

H0R5E commented Sep 29, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants