Skip to content

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
@kmruehl kmruehl requested review from akeeste and kmruehl December 16, 2020 23:41
@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
Collaborator

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
Collaborator

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

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

3 participants