Skip to content

Conversation

@akeeste
Copy link
Contributor

@akeeste akeeste commented Jan 22, 2024

The BEMIO plotting functions were not handling DOFs correctly when not all bodies were being input.
e.g. hydro.plotBodies = 2, then the first body was still being plotted

This PR fixes the logic in each plotting function to accurately plot the hydrodata of bodies called out in hydro.plotBodies, regardless of body order. I also updated some of the looping variables to be more meaningful so that the functions are a bit easier to follow. e.g. iB for body index, iDof for DOF index, etc

@akeeste akeeste added the Bug bug in WEC-Sim source, high priority label Jan 22, 2024
@akeeste akeeste requested a review from jtgrasb January 22, 2024 21:37
@akeeste
Copy link
Contributor Author

akeeste commented Jan 22, 2024

Updated my branch to be based on master

@salhus salhus assigned salhus and jtgrasb and unassigned salhus Jan 24, 2024
@jtgrasb
Copy link
Contributor

jtgrasb commented Jan 30, 2024

Hi Adam, thanks for catching this bug and fixing it. Your fix looks good and works well, and I appreciate the updates to make it more readable as well.

One question I have is for the addDefaultPlotVars.m, is there a point to just adding the plot variables to the last dataset? I don't see an instance where the user would pass in multiple hydro structs, because the readX.m scripts already apply the default plotting variables. Your implementation here works fine with just 1 so no issues with it, I'm just curious.

@akeeste
Copy link
Contributor Author

akeeste commented Jan 30, 2024

Hi @jtgrasb Thanks for the review! On addDefaultPlotVars -- This will break when hydro is an array of structures. For example if I wanted to read multiple output files to compare or combine them:

hydro = readCapytaine(hydro, 'file1.nc', ...)
hydro = readCapytaine(hydro, 'file2.nc', ...)
hydro = combineBEM(hydro)
plotBEMIO(hydro)

Then the second readCapytaine call will fail because addDefaultPlotVars tries to add plotDofs, etc to an array instead of a structure. I was doing this because I was processing a lot of hydro files at once and comparing them.

I think we previously had a set-up like this in compareBEMIO, but it looks like it was changed to save computational time:

% Example of how to combine hydro data structures using combineBEM
hydro(1) = WAMIT_hydro;
hydro(2) = CAP_hydro;
hydroCombined = combineBEM(hydro);

@jtgrasb
Copy link
Contributor

jtgrasb commented Jan 30, 2024

@akeeste Thanks for the explanation, makes sense. Everything looks good then, I will merge this today.

@jtgrasb jtgrasb merged commit ae53996 into WEC-Sim:master Jan 30, 2024
@akeeste akeeste deleted the bugfix_plotBEMIO branch March 20, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug bug in WEC-Sim source, high priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants