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

Start to Translational library with both Velocity and Position based connectors #114

Merged
merged 31 commits into from
Oct 4, 2022

Conversation

bradcarman
Copy link
Contributor

Closes #109

@codecov
Copy link

codecov bot commented Sep 29, 2022

Codecov Report

Merging #114 (516f3ab) into main (28ad75d) will decrease coverage by 3.92%.
The diff coverage is 31.12%.

@@            Coverage Diff             @@
##             main     #114      +/-   ##
==========================================
- Coverage   69.06%   65.13%   -3.93%     
==========================================
  Files          25       32       +7     
  Lines        1225     1394     +169     
==========================================
+ Hits          846      908      +62     
- Misses        379      486     +107     
Impacted Files Coverage Δ
src/Mechanical/MultiBody2D/components.jl 0.00% <0.00%> (ø)
src/Mechanical/Translational/sensors.jl 0.00% <0.00%> (ø)
src/Mechanical/TranslationalPosition/sources.jl 0.00% <0.00%> (ø)
src/Mechanical/TranslationalPosition/utils.jl 4.25% <4.25%> (ø)
src/Mechanical/Translational/components.jl 54.71% <54.71%> (ø)
src/Mechanical/TranslationalPosition/components.jl 61.36% <61.36%> (ø)
src/Mechanical/Translational/utils.jl 75.00% <75.00%> (ø)
src/Thermal/HeatTransfer/ideal_components.jl 100.00% <0.00%> (+2.04%) ⬆️
src/Blocks/math.jl 100.00% <0.00%> (+3.79%) ⬆️
src/Mechanical/Rotational/components.jl 100.00% <0.00%> (+4.34%) ⬆️
... and 5 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Brad Carman and others added 2 commits September 29, 2022 20:52
Copy link
Member

@YingboMa YingboMa left a comment

Choose a reason for hiding this comment

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

Is it intentional to have both s0 and s_0?

@bradcarman
Copy link
Contributor Author

Is it intentional to have both s0 and s_0?

I see this in the sensor component, I've changed to s_0 for the next check in

@bradcarman
Copy link
Contributor Author

About connector naming...there is a problem: when defining a block that combines domains, such as a hydrualic actuator that needs hydraulic and mechanical connectors, we will need to be able to distinguish connectors. For example

function actuator()

    @named H_a = HydraulicPort()
    @named H_b = HydraulicPort()

    @named MT = TranslationalPort()

    eqs = [
        (H_a.p - H_b.p)*area ~ MT.f

        ...
    ]

end

@named act = actuator()

# when making connections we can easily see what like domains are being connected...
eqs = [
    connect(act.H_a, pump.H)
    connect(mass.MT, act.MT)

]

If we decided on a naming scheme now then we can use consistent connector names throughout the Standard Library.

Here is my recommended naming, I am in favor of using capital letters for connectors because they are different from variables, it makes it much easier to read.

#Blocks
B
B_a
B_b

#Electrical/Analog
EA
EA_p #positive
EA_n #negative

#Electrical/Digital
ED
ED_p
ED_n

#Magnetic/FluxTubes
MF
MF_a
MF_b

#Mechanical/Translational
MT
MT_a
MT_b

#Mechanical/Rotational
MR
MR_a
MR_b

#Hydraulic
H
H_a
H_b
H_s #source
H_r #return

@YingboMa
Copy link
Member

What about delta_s0 vs delta_s_0?

@bradcarman
Copy link
Contributor Author

What about delta_s0 vs delta_s_0?

I think the _0 will be best to keep a consistent naming like the others s1_0, s2_0 etc...

@bradcarman
Copy link
Contributor Author

Will stick with flange, flange_a, etc. for now rather than the connector naming I proposed. But I wonder, should I change the initial condition parameters s1_0 to s_a_0 to better align with flange_a?

@YingboMa YingboMa merged commit 0a736e0 into SciML:main Oct 4, 2022
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.

Add Mechanical/Translational libraries with both *position* and *velocity* based connectors
2 participants