-
Notifications
You must be signed in to change notification settings - Fork 184
Viz markers backwards compatible #714
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
Conversation
|
here is the updated PR with enable/disable feature. |
|
@nathanmtom
|
|
@salhus @nathanmtom Other than the small waveClass conflict, is this PR close to merging? I have a couple of library PRs to bring in, one I'd like to do one top of these changes. |
nathanmtom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@salhus Apologies as I needed to look through the file changes a bit more in-depth to understand what is happening. But, once the conflict with waveClass is resolved then this pull request should be good to go. After resolving the conflict let's make sure that we are passing all checks again.
|
@nathanmtom @salhus I was familiar with the conflict (it was with a previous update I made to the wave setup) so I went ahead and resolved it. All tests are still passing after combining the waveClass checks |
|
@akeeste Thanks so much for resolving that ! |
|
@akeeste Thanks for resolving the conflict. @salhus Can you run the WECSimApplications case test file with this pull request? If all checks pass (except the moordyn and paraview checks if you do not have those installed) then I think this can be merged into Dev. Since this feature does not change any force or dynamics calculation we shouldn't have to worry about it affecting simulation outputs. |
…WEC-Sim into VizMarkersBackCompatible
@nathanmtom I ran the tests. It is failing just one test, the WECCOMP. (Attaching the summary) |
|
@salhus @akeeste and @nathanmtom thanks for working on this. I'm going to review it now. |
|
To Do:
NOTE: Question: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@salhus great job on this PR, thank you! I just have two minor questions (above) that we may want to resolve prior to a merge. Also, we need to verify the wec-sim_application tests run and create a new one for this feature.
|
All wec-sim and wec-sim_application tests are passing. |
|
@kmruehl |


This PR updates the functionality in PR #706 .
Corresponding documentation has been updated. Examples have been updated in Applications repo with corresponding documentation.
Here is an example of initializing the visualization blocks. The examples are updated in the Applications Repo.
Examples:
For :code:
mrk = 10;dmrk = 5;[X,Y] = meshgrid(mrk:dmrk:mrk,-mrk:dmrk:mrk);waves.markLoc = [reshape(X,[],1),reshape(Y,[],1)]; % Marker Locations [X,Y]clear('mrk','dmrk','X','Y')waves.markStyle = 3; % 1: Sphere, 2: Cube, 3: Frame.waves.markSize = 20; % Marker Size in Pixels.