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

Migrate @relation to use the ADT approach #837

Open
jpfairbanks opened this issue Aug 17, 2023 · 5 comments
Open

Migrate @relation to use the ADT approach #837

jpfairbanks opened this issue Aug 17, 2023 · 5 comments

Comments

@jpfairbanks
Copy link
Member

Currently this is partially implemented in SyntacticModels. We should backport this to Catlab.

@lukem12345
Copy link
Member

lukem12345 commented Jul 2, 2024

@jpfairbanks Would the closing PR take care of the use case of programmatic construction?

There is code in DiagrammaticEquations.jl that has to do metaprogramming to use the constructor. But it would be great if equivalent functionality was offered upstream.

@jpfairbanks
Copy link
Member Author

The code you linked could just use the programmatic interface for the ACSet as it already is.

I haven't test this, but it should be pretty close.

using Catlab
using Catlab.Programs
using Catlab.RelationalPrograms
using Catlab.WiringDiagrams.Undirected

function construct_relation_diagram(boxes::Vector{Symbol}, junctions::Vector{Vector{Symbol}})
  d = UntyledRelationDiagram{Symbol, Symbol}()
  juncs = unique(flatten(junctions))
  map(juncs) do j
    add_junctions!(d, variable=j)
  end
  map(boxes, junctions) do b, js
    # Expr(:call, b, j...)
    box = add_box!(d, b, js...)
    ports = incident(d, box, :port)
    map(ports, js) do p, j
      set_junction!(d, p, j)
    end
  end
  return d
  # quote @relation () begin $(tables...) end end |> eval
end

@jpfairbanks
Copy link
Member Author

Ok, I also looked at what I had in SyntacticModels, and it is a little more complicated to get the outer ports and variable types right.

https://github.com/AlgebraicJulia/SyntacticModels.jl/blob/main/src/uwd.jl#L158

Yes, once this refactor is implemented you should have an easier time writing constructors like this that don't use macros because you can build a UWDExpr programmatically, rather than constructing the julia expression.

@lukem12345
Copy link
Member

lukem12345 commented Jul 2, 2024

Yes as far as I can remember, there is another potential footgun due to the fact that the mapping from global ports to local ports depends on the order of rows in the Ports table. This came up in the context of implementing oapply for Decapodes (which ultimately avoided the issue due to not needing to make use of outer ports.) I think that code that needs that info is essentially relying on an “implementation detail” kind of behavior, though the lines get blurry.

@jpfairbanks
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants