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

Generated code for composites of C-set subparts #9

Closed
epatters opened this issue Jan 18, 2021 · 9 comments
Closed

Generated code for composites of C-set subparts #9

epatters opened this issue Jan 18, 2021 · 9 comments

Comments

@epatters
Copy link
Member

epatters commented Jan 18, 2021

Composites of C-set subparts can be accessed using

subpart(x, i, [:f, :g])

Because the list of subparts is a vector, it must be map-reduced at runtime. We should also support a static version, based on tuples, that @generates code:

subpart(x, i, (:f, :g))

Likewise for the function incident.

This is motivated by AlgebraicJulia/Catlab.jl#371, where composites of length 2 are used often in the implementation of DWDs as ACSets. When this feature is implemented, all the DWD code should be updated accordingly.

@epatters epatters changed the title Generated code for composites of subparts Generated code for composites of C-set subparts Jan 18, 2021
@epatters epatters transferred this issue from AlgebraicJulia/Catlab.jl May 30, 2023
@slwu89
Copy link
Member

slwu89 commented Feb 12, 2024

@epatters I am working on addressing this (for both subpart and incident) at the moment. During that work, I noticed that incident(acs::StructACSet{S}, ::Colon, f::Symbol) does not return anything if f corresponds to an attribute rather than a morphism between objects. The reason is that the following code doesn't return the values stored in the Attribute (and by extension, neither does codom_parts). Is this intended behavior? If it is not, I am happy to address it in the course of working on this issue.

julia> datcomp
CompositesData{Symbol} {X:5, Y:4, Z:3, W:3, Zattr:0}
┌───┬───┬───┐
│ X │ f │ h │
├───┼───┼───┤
│ 111 │
│ 222 │
│ 333 │
│ 411 │
│ 522 │
└───┴───┴───┘
┌───┬───┐
│ Y │ g │
├───┼───┤
│ 13 │
│ 22 │
│ 31 │
│ 43 │
└───┴───┘
┌───┬───────┐
│ Z │ zattr │
├───┼───────┤
│ 1 │     a │
│ 2 │     b │
│ 3 │     c │
└───┴───────┘


julia> parts(datcomp, codom(acset_schema(datcomp), :zattr))
1:0

@epatters
Copy link
Member Author

Thanks for working on this, Sean!

I am wondering what the intended behavior of, say, incident(g, :, :weight) when g is a weighted graph, should even be. The colon usually means "all of the elements of a list/FinSet" but the attribute type is effectively infinite. Did you have something in mind?

@slwu89
Copy link
Member

slwu89 commented Feb 13, 2024

Ah I was thinking that : was returning elements in the codom that were hit by the function. But on second thought the current interpretation makes much more sense and avoids strange issues.

@jpfairbanks
Copy link
Member

My first reaction to incident(g, :, :weight) is that it it should be like {y => incident(g, y, :weight) for y in g[:, :weight]} because the codomain of :weight is infinite, you can't return the whole inverse function, only the sparsely defined part. And since the codomain of :weight isn't canonically isomorphic to 1:n, you shouldn't return a list like[incident(g, y, :weight) for y in g[:, :weight]]. So either a list of pairs or a Dict makes the most sense to me.

@slwu89
Copy link
Member

slwu89 commented Feb 13, 2024

@jpfairbanks that makes sense, although I wonder if that would introduce too much complexity into the possible types of outputs from incident. And what should happen if someone did incident(g, :, [:src,:weight])?

@jpfairbanks
Copy link
Member

I think the composite case should be handled by asking, what would I do with the composite function and then decide how to implement that. I would expect a map from each weight in the image of src;weight to the set of edges whose src has that weight.

@slwu89
Copy link
Member

slwu89 commented Feb 13, 2024

Great! that's what I ended up doing in #109, for composites. Didn't change anything for the single attr (arrow into typeset) case.

@slwu89
Copy link
Member

slwu89 commented Feb 16, 2024

I think this one is good to close since #109 is merged, however @jpfairbanks did bring up the interesting point of what should happen when someone does incident(g, :, :weight), so I don't know if we want to rename the issue and keep it around for that discussion, or close this issue and move that elsewhere.

@epatters
Copy link
Member Author

IMO, that should be a separate issue, although you could link the discussion here if helpful. Thanks for closing a longstanding issue with acsets!

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