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.

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

PTO-Sim library update resolves #247 #485

Merged
merged 1 commit into from Dec 17, 2020

Conversation

jleonqu
Copy link
Contributor

@jleonqu jleonqu commented Dec 16, 2020

This PR corrects the Rotary to Linear blocks in the PTO-Sim. The torque calculation is corrected in both blocks, the formulation was briefly described in the issue #247

The blocks revised are the Motion Conversion blocks in the PTO-Sim library. The changes are highlighted in the next figure:

image

@kmruehl kmruehl added PTO-Sim PTO-Sim (ptoSimClass.m) Bug bug in WEC-Sim source, high priority labels Dec 16, 2020
@akeeste
Copy link
Contributor

akeeste commented Dec 17, 2020

Hi @jleonqu

I'm having a couple issues running this case. I think that in the PTO-Sim library blocks a few variables were renamed. For example it looks like ptosim.motionMechanism.crank became ptosim.crank, etc. However the ptosimClass was not updated to reflect this naming that so I can't run the case yet. Can you also commit/push the updated ptosimClass.m file? Then I will test your PR again.

Adam

@jleonqu
Copy link
Contributor Author

jleonqu commented Dec 17, 2020

Hi @akeeste

It's kind of weird, because I didn't change the names in the blocks. This is how the block looks like originally:
image

And this is how it looks like after my changes:
image

I used the same notation. The original variable name is ptosim.crank in the blocks and I used the same name for updated version.

Jorge.

@akeeste
Copy link
Contributor

akeeste commented Dec 17, 2020

@jleonqu I see, this is strange. In the current ptosimClass.m on dev I see these variables under the ptosim.motionMechanism struct. When I tried to add them to the ptosimClass itself in the ptoSimInputFile:

ptosim.crank = 3;
ptosim.offset = 1.3;
ptosim.rodInit = 5;

I get the error:
"Unrecognized property 'crank' for class 'ptoSimClass'.

Error in ptoSimInputFile (line 6)
ptosim.crank = 3;

Error in wecSim (line 136)
ptoSimInputFile"

@jleonqu
Copy link
Contributor Author

jleonqu commented Dec 17, 2020

@akeeste it looks like in the ptoSimInputFile you should add the variables like this:

ptosim.motionMechanism.crank = 3;
ptosim.motionMechanism.offset = 1.3;
ptosim.motionMechanism.rodInit = 5;

I just checked a couple of examples in the WEC-Sim_Applications repository, and that's the notation.

@kmruehl
Copy link
Contributor

kmruehl commented Dec 17, 2020

@akeeste it looks like in the ptoSimInputFile you should add the variables like this:

ptosim.motionMechanism.crank = 3;
ptosim.motionMechanism.offset = 1.3;
ptosim.motionMechanism.rodInit = 5;

I just checked a couple of examples in the WEC-Sim_Applications repository, and that's the notation.

@jleonqu, do we need to change the inputs to the PTO-Sim example on the applications repository too, or are they unchanged?

@jleonqu
Copy link
Contributor Author

jleonqu commented Dec 17, 2020

@kmruehl the variables are unchanged, this is the original notation used in the ptoSimInputFile in all the application cases.

@kmruehl
Copy link
Contributor

kmruehl commented Dec 17, 2020

This PR corrects the Rotary to Linear blocks in the PTO-Sim. The torque calculation is corrected in both blocks, the formulation was briefly described in the issue #247

@jleonqu can you modify this description to include a screenshot of the blocks you revised, since GitHub doesn't easily track changes to binary likes, like the Simulink Library. Thanks!

Copy link
Contributor

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

@jleonqu thank you so much for getting to the bottom of this issue! I was able to run the OSWEC_Hydraulic_Crank_PTO application case and confirm that this PR resolves the sign error in the kinematic linkage. Plotting the same figure as in #247 below, but with sign error resolved.

ptosim

figure()
plot(output.ptos.time,output.ptos.forceActuation(:,5))
hold on;
plot(output.ptos.time,output.ptos.forceInternalMechanics(:,5)) 
hold on;
plot(output.ptosim.time,output.ptosim.pistonNCF.force)
legend('Actuation Torque','Internal Torque','PTO-Sim Torque')

@kmruehl kmruehl changed the title PTO-Sim library update PTO-Sim library update resolves #247 Dec 17, 2020
@akeeste
Copy link
Contributor

akeeste commented Dec 17, 2020

Hi @jleonqu @kmruehl I found my issue. Sorry this is not actually related to what you implemented in this PR, but to a strange way that the motionMechanism mask was set-up previously that may be worth changing to prevent confusion.

In my case, I was using both the "Rotary to Linear Adjustable Rod" and the "Angular to Linear Velocity" blocks from PTO-Sim. Normally, "Angular to linear velocity" is normally contained within the mask of "Rotary to Linear Adjustable rod". The mask takes a "ptosim.motionMechanism" struct as an input, but assigns it to the variable "ptosim" under the mask. This was very confusing as then it appears that the blocks are using ptosim.crank, ptosim.rodInit, etc. I think if the mask instead assigned the input to "ptosim.motionMechanism" or just "motionMechanism" this would prevent confusion.

I setup simulink this way because I only needed to convert an angular velocity, not an additional force. I can work around this another way, but wanted to point out where my confusion was coming from.

image

@jleonqu
Copy link
Contributor Author

jleonqu commented Dec 17, 2020

@kmruehl I updated the description of the PR to include a figure of the blocks that were revised.

Copy link
Contributor

@akeeste akeeste left a comment

Choose a reason for hiding this comment

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

Approved!

@kmruehl kmruehl merged commit eb17d45 into WEC-Sim:dev Dec 17, 2020
@jleonqu jleonqu deleted the Feature_Kinematics branch May 23, 2023 16:37
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 PTO-Sim PTO-Sim (ptoSimClass.m)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants