Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Multi wave class bug fix v2 #1373

Merged
merged 5 commits into from
Dec 3, 2024
Merged

Multi wave class bug fix v2 #1373

merged 5 commits into from
Dec 3, 2024

Conversation

MShabara
Copy link
Contributor

@MShabara MShabara commented Nov 19, 2024 •
edited
Loading

This is a replacement for PR #1371

This PR maintains the usage of the "GotoTag" tags within the "Linear Wave Excitation Force Variant Subsystem" sub-blocks which is needed for the variable-hydro feature.

The Modified file: The mask initialization for the "Wave Diffraction and Excitation Force Calculation" block. The following lines were added:


set_param([cb, '/Linear Wave Excitation Force Variant Subsystem', num2str(iWG)], 'LinkStatus', 'inactive');
     .
     . 
     .
% body_hydroForce From tags
h=[cb '/Linear Wave Excitation Force Variant Subsystem',num2str(iWG),'/Regular Wave  Excitation Force/From'];
responseName=['body' num2str(body.number) '_hydroForce'];
set_param(h,'GotoTag',responseName);
h=[cb '/Linear Wave Excitation Force Variant Subsystem',num2str(iWG),'/Regular Wave  NonLinear Yaw/From'];
set_param(h,'GotoTag',responseName);
h=[cb '/Linear Wave Excitation Force Variant Subsystem',num2str(iWG),'/Irregular Wave  Excitation Force/From'];
responseName=['body' num2str(body.number) '_hydroForce'];
set_param(h,'GotoTag',responseName);
h=[cb '/Linear Wave Excitation Force Variant Subsystem',num2str(iWG),'/Irregular Wave  NonLinearYaw/From'];
responseName=['body' num2str(body.number) '_hydroForce'];
set_param(h,'GotoTag',responseName);

Note that the first line is essential to allows library overrides. Without this line the mask initializer won't be able to modify the GotoTags

@MShabara MShabara requested a review from akeeste November 19, 2024 23:46
@MShabara MShabara mentioned this pull request Nov 19, 2024
Closed
@kmruehl kmruehl added the Bug bug in WEC-Sim source, high priority label Nov 20, 2024
@akeeste
Copy link
Contributor

akeeste commented Dec 2, 2024

@MShabara This method works for me, but with the unintended consequence that the top-level library link also gets broken every simulation, which may be misleading for users.

I have another idea about relocating the variable hydro From block to the Wave Diffraction and Excitation Force Calculation level. Then there is only one From block tag that the mask needs to change and it can be connected to multiple excitation force blocks from multiple waves like the other inputs are. I'll test and push a commit later today if I get this working.

On another note, does QTF currently work with multiple waves? In another PR We may want to change the definition of waveAmpTime in bodyClass.hydroForcePre to be the combined wave elevation time series from all defined waves.

@akeeste
Copy link
Contributor

akeeste commented Dec 2, 2024

I have another idea about relocating the variable hydro From block to the Wave Diffraction and Excitation Force Calculation level. Then there is only one From block tag that the mask needs to change and it can be connected to multiple excitation force blocks from multiple waves like the other inputs are.

@MShabara I got this method working and doesn't break the top-level links now. If you approve and tests pass, I'm good to merge this PR

@MShabara
Copy link
Contributor Author

MShabara commented Dec 2, 2024 •
edited
Loading

@akeeste I have verified the updates, and the body library behaves as expected. The results from a single wave instance match those from multiple wave instances. These changes are ready to merged.

For the QTFs: I have modified the initialization function and the body class to account for handling multiple wave instances. If you approve, I'm good to merge this PR

@MShabara MShabara added the QTFs label Dec 2, 2024
@akeeste
Copy link
Contributor

akeeste commented Dec 2, 2024

thanks @MShabara the QTF update looks good. if there's no needed change to the .h5 file, I'll remove that and merge

@akeeste
Copy link
Contributor

akeeste commented Dec 3, 2024 •
edited
Loading

I reverted the change to the h5 file and tests are passing. Merging into main. This eventually needs to be pulled into dev, perhaps after #1332

@akeeste akeeste merged commit bdad54a into WEC-Sim:main Dec 3, 2024
10 checks passed
@akeeste akeeste mentioned this pull request Dec 11, 2024
Merged
This was referenced Dec 17, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akeeste akeeste Awaiting requested review from akeeste

Assignees

@akeeste akeeste

Labels
Bug bug in WEC-Sim source, high priority QTFs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MShabara @akeeste @kmruehl