Skip to content

Conversation

@salhus
Copy link
Contributor

@salhus salhus commented Sep 23, 2021

This PR updates the functionality in PR #706 .

Corresponding documentation has been updated. Examples have been updated in Applications repo with corresponding documentation.

  • Updated the initialization such that if user defines the array of marker coordinates, visualization is initialized,
  • If the user does not define the coordinates as an Nx2 array, the user will get the following error message.

image

Here is an example of initializing the visualization blocks. The examples are updated in the Applications Repo.
Examples:

  1. An example with a square mesh of visualization blocks:
    For :code:
    mrk = 10;
    dmrk = 5;
    [X,Y] = meshgrid(![image](https://user-images.githubusercontent.com/84348506/134778002-256e3c0d-547a-4cc3-888f-7bdddf65f053.png)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.

@salhus
Copy link
Contributor Author

salhus commented Sep 23, 2021

@nathanmtom @kmruehl

here is the updated PR with enable/disable feature.

@salhus salhus mentioned this pull request Sep 23, 2021
@salhus
Copy link
Contributor Author

salhus commented Sep 25, 2021

@nathanmtom
As discussed on Friday during open-office hours. I updated the PR to have,

  • Remove the initialization using a variable. Now the user only needs to define the Nx2 array,
  • If the array dimensions are incorrect the user will get an error message,
  • Updated the applications examples to have @jtgrasb 's visualization in the user-defined-functions file,
  • Updated the corresponding docs.

@akeeste
Copy link
Contributor

akeeste commented Sep 27, 2021

@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.

Copy link

@nathanmtom nathanmtom left a 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.

@akeeste
Copy link
Contributor

akeeste commented Sep 28, 2021

@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

@salhus
Copy link
Contributor Author

salhus commented Sep 28, 2021

@akeeste Thanks so much for resolving that !

@nathanmtom
Copy link

@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.

@salhus
Copy link
Contributor Author

salhus commented Sep 28, 2021

@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.

@nathanmtom I ran the tests. It is failing just one test, the WECCOMP. (Attaching the summary)
testsAppRepo.txt

@kmruehl
Copy link
Collaborator

kmruehl commented Sep 28, 2021

@salhus @akeeste and @nathanmtom thanks for working on this. I'm going to review it now.

@kmruehl
Copy link
Collaborator

kmruehl commented Sep 28, 2021

To Do:

  • clean up variable naming
  • move Frames to sub-library
  • save library to 2020b
  • update documentation
  • run wec-sim_application tests

NOTE:
The mask initialization is not compatible with 2020a this is similar to the issue I mentioned with B2B on #688. For this PR the library will now need to be saved in 2020b

Question:

  • Why are there dialogue boxes for masks within the Frame block?
    image
    image

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.

@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.

@kmruehl
Copy link
Collaborator

kmruehl commented Sep 29, 2021

All wec-sim and wec-sim_application tests are passing.

@salhus
Copy link
Contributor Author

salhus commented Sep 29, 2021

@kmruehl
Thanks for the review. I had the dialog boxes as a way to know what settings were selected by the user. It also helped me do the mask initialization when doing the initialization for the variable subsytems without using a 'comment/uncomment' code which older matlabs didnt support.
Let me know if that answers the question..

@kmruehl kmruehl merged commit fc34536 into WEC-Sim:dev Sep 30, 2021
@salhus salhus deleted the VizMarkersBackCompatible branch October 1, 2021 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants