Skip to content

add ClockChange operator #2744

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

Closed
wants to merge 8 commits into from
Closed

add ClockChange operator #2744

wants to merge 8 commits into from

Conversation

baggepinnen
Copy link
Contributor

@baggepinnen baggepinnen commented May 27, 2024

This PR adds a discrete-time operator ClockChange that changes the clock of a discrete-time variable. This allows for sub and super sampling.

Status: In the simple test model added to the tests, I get an error from the compiler

julia> @mtkbuild cc = CC()
ERROR: Some specified inputs were not found in system. The following variables were not found Any[ModelingToolkit.ClockChange(Clock(t, 2.0), Clock(t, 1.0))(y(t))]
Stacktrace:
  [1] error(::String, ::Base.KeySet{Any, Dict{Any, Bool}})
    @ Base ./error.jl:44
  [2] markio!(state::TearingState{ODESystem}, orig_inputs::Set{Any}, inputs::Vector{Any}, outputs::Vector{Any}; check::Bool)
    @ ModelingToolkit ~/.julia/dev/ModelingToolkit/src/systems/abstractsystem.jl:2038
  [3] markio!
    @ ~/.julia/dev/ModelingToolkit/src/systems/abstractsystem.jl:2009 [inlined]
  [4] _structural_simplify!(state::TearingState{…}, io::Tuple{…}; simplify::Bool, check_consistency::Bool, fully_determined::Bool, warn_initialize_determined::Bool, dummy_derivative::Bool, kwargs::@Kwargs{})
    @ ModelingToolkit ~/.julia/dev/ModelingToolkit/src/systems/systemstructure.jl:680

The problem appears realted to the hacky forward shift employed by the compiler, since

fullvars[end]
ModelingToolkit.ClockChange(Clock(t, 2.0), Clock(t, 1.0))(Shift(t, 2)(y(t)))

while the operator variable is not shifted

collect(keys(inputset))[]
ModelingToolkit.ClockChange(Clock(t, 2.0), Clock(t, 1.0))(y(t))

EDIT: I pushed a commit that applies the shift to the new operator as well. The remaining problem is that x is updated before y, while it should be the opposite, the equation for x depends on y and has to come after.

@baggepinnen
Copy link
Contributor Author

@aayush Sabharwal @yingbo Ma
It looks like we need to take some time to clean up the current implementation that forward-shifts discrete time variables, it leads to headaches like above.

I'd be in favor of getting rid of the forward-shift hack completely, are there any fundamental reasons left that prevents us from doing that?

@YingboMa YingboMa closed this Jun 18, 2024
@YingboMa YingboMa deleted the fb/clockchange branch June 18, 2024 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants