-
Notifications
You must be signed in to change notification settings - Fork 184
Body Library Refactoring #1474
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
Body Library Refactoring #1474
Conversation
|
Adam TODO before deferring some TODO items to another PR:
|
|
@kmruehl this PR is ready for review. I was able to get the few top-level mask initialization functions moved to .m files, but not the rest. I believe all of the applications cases run. The regression tests fail but I don't know their most recent status. I was not able to update documentation yet to reflect the removal of the nonhydro body |
|
Thanks @akeeste I'm making a few revisions to the library:
|
kmruehl
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.
@akeeste thanks for working on this. I made some minor updates that broke then library. Let's debug when you're back in the office and discuss next steps
|
Okay, after discussion between @akeeste and @kmruehl, this PR has changed scope significantly and now does the following:
The body types are no longer split into multiple files because it does not add value and increases burden on the user |
|
@akeeste is this ready for review? |
I am actually having this issue on this PR, main and dev, so it must be an issue on my end. Furthermore, @jtgrasb confirmed that he does not have this issue viewing the library. I'm going to reinstall MATLAB, and upgrade to 2025a. |
|
@kmruehl Yes this is ready for review. I'm not sure what's causing the library appearance issue. On R2024b PTO-Sim renders fine but not the rest. I agree that maybe it's a MATLAB version problem |
|
@akeeste that's the same for me with 2024a. PTO-Sim renders properly, but nothing else does. Everything renders properly for me on 2025a and @jtgrasb confirmed that 2023b renders properly, so it must be an issue with the 2024a and 2024b distributions. I don't think it's worth debugging because the library blocks still work, they just don't render in the library. I'm just mentioning it in this thread for future awareness. |
kmruehl
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.
@akeeste thanks for drafting this PR, but I think it needs some work prior to merging. Several of the docs need to be updated, and so does initializeWecSim and bodyClass. Right now I am not able to run the RM3 from Simulink, and the docs for the Body Library updates are confusing.
|
@akeeste I made some very minor updates towards the requested changes. Let's chat about how to get these revisions done and merging the PR. Thanks! Also, why are the tests failing? |
|
Thanks @kmruehl I'll make the requested changes. I don't know why the tests are failing. They have not run consistently for a while and the regression were only recently fixed |
|
It looks like the tests are not running to completion, I'll look into that too |
|
I'm not sure why there tests are having issues that they weren't before, but I also fixed that. They were failing on the 2nd regression test because the RM3 simulink file from the previous one was still open, leading to conflicts with open models on the path. I force close all simulink models after each regression test |
|
Tests finally passing. Merged |
|
Thanks @akeeste |


Addresses part of the project card here: https://github.com/orgs/WEC-Sim/projects/12/views/1?pane=issue&itemId=91263089
This PR refactors the body library:
To update applications to work with this PR:
TODO