fixed force sign

[bradcarman]

The correct force sign convention was not implemented properly. The convention should be, using the mass component for example, to draw the port orientation and the component orientations, then variables that are in opposite direction should have a negative sign. For the mass component, if the movement is from left to right, then the force will push back from right to left, which is in opposite direction of the port orientation.

This convention is now applied throughout the Mechanical/Translational library.

As a quick proof, we can define a ConstantForce and Mass component. We would expect connecting the 2, if a positive force is given, the mass should increase in velocity, and vice versa. The output of the system below, implementing this sign convention, gives:

julia> full_equations(sys)
1-element Vector{Equation}:
 Differential(t)(mass₊v(t)) ~ force₊f / mass₊m

As can be seen, a positive input force force₊f will result in a positive increase in velocity mass₊v

Proof System:

using ModelingToolkit

@parameters t
D = Differential(t)

@connector MechanicalPort begin
    v(t)
    f(t), [connect = Flow]
end

@mtkmodel ConstantForce begin
    @parameters begin
        f
    end
    @components begin
        port = MechanicalPort()
    end
    @equations begin
        # connectors
        port.f ~ f  
    end
end

@mtkmodel Mass begin
    @parameters begin
        m = 10
    end
    @variables begin
        v(t)
        f(t)
    end
    @components begin
        port = MechanicalPort()
    end
    @equations begin
        # connectors
        port.v ~ v
        port.f ~ -f
        
        # physics
        f ~ m*D(v)
    end
end

@mtkmodel System begin
    @components begin
        mass = Mass()
        force = ConstantForce()
    end
    @equations begin
        connect(mass.port, force.port)
    end
end
@mtkbuild sys = System()

[codecov]

Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (f46cdc0) 55.48% compared to head (c86e702) 17.01%.
Report is 2 commits behind head on main.

Files	Patch %	Lines
src/Hydraulic/IsothermalCompressible/components.jl	0.00%	29 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #246       +/-   ##
===========================================
- Coverage   55.48%   17.01%   -38.48%     
===========================================
  Files          48       48               
  Lines        1642     1646        +4     
===========================================
- Hits          911      280      -631     
- Misses        731     1366      +635     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[baggepinnen]

Even if this is a fix of the convention, it's obviously a massively breaking change for anyone that has implemented a model using the changed components and made it work with the opposite convention. If we merge this, we'd have to bump the major version

[bradcarman]

Agreed. Is there a way to tag this for a major version change?

[baggepinnen]

Changing the version is done by modifying the version specification in Project.toml, followed by triggering the registrator bot. @ven-k usually does this for this package IIRC?

[ven-k]

I can't trigger the registration bot for MSL.

While here, should we have a next or release-3.0 branch?

[ChrisRackauckas]

MTK is already master to v9. We can make master here be set for now the new major, and do the v9 update as part of that.

[bradcarman]

@baggepinnen can you help me with the analysis points failure? This doesn't seem to be related to the change.

[baggepinnen]

The test passes locally for me. The analysis point code hasn't been touched in several months, it might be a change related to SymbolicIndexInterface that has broken the test?

[baggepinnen]

The latest integration test for MTKstdlib that is run as part of the test suite of MTK passed, so it's probably no related to the indexing

[baggepinnen]

It looks like there has been a small numerical change to some solver, the test passes if I change

rtol=1e-4

to

atol=1e-3

[baggepinnen]

Actually scratch that, if I call Array on the array that is returned by sol, it passes

julia> @test stepres.y[:]≈Array(sol(0:0.001:4, idxs = model.inertia2.phi)) rtol=1e-4
Test Passed

so it might be that something has changed in how this array type is compared to normal arrays

julia> sol(0:0.001:4, idxs = model.inertia2.phi) |> typeof
RecursiveArrayTools.DiffEqArray

[bradcarman]

Thanks @baggepinnen ! I'll integrate the change.

[bradcarman]

Closing because this sign convention is actually not uniform across Electrical, Mechanical, etc. And is the opposite of the Modelica standard. Opening a new PR with the correct sign convention.