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

Bug in Open for variables that are derivatives #134

Closed
jpfairbanks opened this issue Jul 27, 2023 · 5 comments · Fixed by #136
Closed

Bug in Open for variables that are derivatives #134

jpfairbanks opened this issue Jul 27, 2023 · 5 comments · Fixed by #136
Assignees
Labels
bug Something isn't working

Comments

@jpfairbanks
Copy link
Member

It looks like if you want to expose a variable that is a derivative, you can't.

MWE:

using Test
using Decapodes
using Catlab

dexpr = Decapodes.parse_decapode(quote
  X::Form0{Point}
  V::Form0{Point}

  k::Constant{Point}

  ∂ₜ(X) == V
  ∂ₜ(V) == -1*k*(X)
end
)

A = dom(legs(Open(SummationDecapode(dexpr), [:V]))[1])
@test nparts(A, :Var) == 1
@jpfairbanks jpfairbanks added the bug Something isn't working label Jul 27, 2023
@lukem12345
Copy link
Member

lukem12345 commented Jul 27, 2023

This is because we do a renaming by appending dots. e.g. Here, V becomes X \dot \dot.

julia> SummationDecapode(dexpr)
SummationDecapode{Any, Any, Symbol} with elements Var = 1:6, TVar 
= 1:2, Op1 = 1:2, Op2 = 1:2, Σ = 1:0, Summand = 1:0, Type = 1:0, Operator = 1:0, Name = 1:0
┌─────┬──────────┬────────┐ 
│ Var │     type │   name │ 
├─────┼──────────┼────────┤ 
│   1 │    Form0 │      X │ 
│   2 │    Form0 │      Ẋ │ 
│   3 │ Constant │      k │ 
│   4 │    infer │ mult_1 │ 
│   5 │    infer │      Ẋ̇ │
│   6 │  Literal │     -1 │ 
└─────┴──────────┴────────┘ 
┌──────┬──────┐
│ TVar │ incl │
├──────┼──────┤
│    12 │
│    25 │
└──────┴──────┘
┌─────┬─────┬─────┬─────┐
│ Op1 │ src │ tgt │ op1 │
├─────┼─────┼─────┼─────┤
│   112 │  ∂ₜ │
│   225 │  ∂ₜ │
└─────┴─────┴─────┴─────┘
┌─────┬───────┬───────┬─────┬─────┐
│ Op2 │ proj1 │ proj2 │ res │ op2 │
├─────┼───────┼───────┼─────┼─────┤
│   1634* │
│   2415* │
└─────┴───────┴───────┴─────┴─────┘

@lukem12345
Copy link
Member

@jpfairbanks

The renaming-with-dot behavior should get changed, but just to ascertain whether composition of derivatives works, does using X dot dot instead of V in your MWE work with whatever composition you were attempting?

@jpfairbanks
Copy link
Member Author

Yes, that workaround works. I guess this is a general problem about special casing the decapodes implementation to the DEC. Maybe we should refactor out the diagrammatic equations part and then have the parts that assume special structure of DEC as postprocessing.

@lukem12345
Copy link
Member

I think the real reason here is that sometimes users write \partial\_t(foo) = d(bar). But they never provide a human-readable name for what to call this quantity, so we rename it to foo dot. This renaming is too aggressive, but not directly related to anything DEC. But yes such a refactoring should be considered.

@lukem12345
Copy link
Member

Calling this quantity X dot dot gives the human some information that this is a double derivative without having to do any analysis of the TVar table. Calling it V respects the way that the user specified variable names in the macro. Both naming conventions are nice in their own way. The latter should probably be preferred since the former loses information. In the PR that closes this, I want to provide a function that does the dot renaming in its current aggressive form, but do the respectful non-renaming by default. Best of both worlds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants