Skip to content

Conversation

@jtgrasb
Copy link
Contributor

@jtgrasb jtgrasb commented Mar 13, 2025

This PR addresses #1438 which found a bug in the paraview output function. If simu.paraview.dt is larger than simu.dt, the paraview timestep will only be applied to the first body. To fix this, I moved the if statement to before the bodies are looped through and moved the time vector selection outside of the if statement so it is applied for all bodies.

Thanks for finding this issue and suggesting a fix @hamzaJayy.

@kmruehl kmruehl self-assigned this Mar 13, 2025
@kmruehl kmruehl self-requested a review March 13, 2025 17:03
@jtgrasb
Copy link
Contributor Author

jtgrasb commented Mar 17, 2025

@kmruehl I also amended our post-processing for the mooring class and the Paraview functions to account for multiple mooring connections. This should be working now with all data included in the output structure and the Paraview folder when turned on.

One thing that is not set up is loading multiple mooring connections into Paraview. The WEC-Sim.py script is set up for the RM3_MoorDyn example which only has 1 MoorDyn connection, so it only reads in the one. I propose that we leave WEC-Sim.py as an example for a Paraview input script and suggest that users create their own as needed, since we don't have anyone on the team that actively uses Paraview.

@kmruehl
Copy link
Collaborator

kmruehl commented Apr 2, 2025

Based on conversation with @jtgrasb I reverted this PR to only resolve the bug in #1438, and moved the MoorDyn feature development to #1455 to address #1440 .

@kmruehl kmruehl force-pushed the paraview_fix_main branch from 6103046 to 4a9e413 Compare April 2, 2025 22:25
@kmruehl
Copy link
Collaborator

kmruehl commented Apr 2, 2025

To very this feature take the RM3 case and set simu.paraview.option = 1 and simu.paraview.dt > simu.dtOut > simu.dt and checked the resulting timestep in the ParaView output files

@kmruehl
Copy link
Collaborator

kmruehl commented Apr 2, 2025

@jtgrasb I'm getting the following error when I run the RM3_MoorDyn_Viz applications case with simu.paraview.dt = 0.5. The case ran without issue when I didn't specify simu.paraview.dt

Index exceeds the number of array elements. Index must not exceed 161.

Error in writeParaviewMooring (line 60)
            pt = [moorDyn.(['Line' num2str(iline)]).(['Node' num2str(inode) 'px'])(it), moorDyn.(['Line' num2str(iline)]).(['Node' num2str(inode) 'py'])(it), moorDyn.(['Line' num2str(iline)]).(['Node' num2str(inode) 'pz'])(it)];

Error in paraviewVisualization (line 62)
        writeParaviewMooring(output.moorDyn, modelName, output.moorDyn.Lines.Time, datestr(simu.date), mooring.moorDynLines, mooring.moorDynNodes,simu.paraview.path,TimeBodyParav,NewTimeParaview)

Error in stopWecSim (line 38)
paraviewVisualization

Error in run (line 112)
evalin('caller', strcat(scriptStem, ';'));

Error in wecSim (line 56)
run('stopWecSim');

@kmruehl kmruehl added Visualization Visualization and Paraview Simulation Class Simulation Class (simulationClass.m) labels Apr 2, 2025
@kmruehl
Copy link
Collaborator

kmruehl commented Apr 3, 2025

Disregard above error. I was working from main instead of paraview_fix_main. Your PR resolves the multibody issue, thank you!

In running this case I noticed and resolved another bug. We should be removing the vtk directory between WEC-Sim runs writing ParaView output. The reason is that if you run, for example, a simulation with simu.paraview.dt = 0.1 yielding 800 output files and then a run with simu.paraview.dt = 0.5 yielding 160 output files, only the first 160 files are overwritten, and the remaining 640 are from the old run. This is not only confusing, but it can also cause issues when you run ParaView for visualization. We already had a try/catch statement in paraviewVisualization.m to remove the vtk directory, but there was a path error which I've resolved in this updated PR. @jtgrasb can you verify that this works for you then we're ready for a merge. Thanks!

Copy link
Collaborator

@kmruehl kmruehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ready for a merge once @jtgrasb confirms the remove vtk dir

@jtgrasb
Copy link
Contributor Author

jtgrasb commented Apr 4, 2025

Also added a couple variables to the clear statement to avoid errors if you run a model with Paraview twice in a row with different settings. Merging once tests pass.

@jtgrasb jtgrasb merged commit 62b5b34 into WEC-Sim:main Apr 4, 2025
10 checks passed
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 Simulation Class Simulation Class (simulationClass.m) Visualization Visualization and Paraview

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants