Higher level incident call #60
Conversation
Added graph examples with more complicated uses
These functions are meant as basic decapodes querying functions
|
Some calls may be better off as |
|
I'd rather not be writing Having this in ACSets main sounds fine but it leads me to wonder what exactly is supposed to be the difference between ACSets and DiagrammaticEquations. |
|
To ACSets.jl belong data structures and algorithms over generic (not only compute-graph like) schemas. Partitioning the code as of the most recent commit, The "query" related code is quite similar to the chained-subparts code, which Sean contributed to ACSets.jl, in that case using CompTime. A recent draft PR demonstrates how using Sean's PR provides syntactic sugar for nested calls to |
The array/append! method before would always return Vector{Any}, which didn't match incident's behavior.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #60 +/- ##
==========================================
- Coverage 86.80% 86.55% -0.25%
==========================================
Files 13 15 +2
Lines 894 922 +28
==========================================
+ Hits 776 798 +22
- Misses 118 124 +6 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
@GeorgeR227, could we replace some of this with calls to querry from Catlab?
It lets you use the relation macro as a declarative DSL for queries.
Since most queries are a chain of subpart and incident calls, you just write those as simple relations.
If you want to find all the variables that are computed by a direct circularity.
q = @relation begin
Op1(src=v, tgt=u)
Op1(src=u, tgy=v)
end
d = SummationDecapode(...)
query(d, q)This would match patterns like u = f(v); v = g(u). But then you can also do complex patterns like:
@relation begin
Op1(src=v, tgt=u)
Op2(proj1=v, proj2=u, res=x)
endThis is a pattern of a triangular dependency where what looks like an op2 on v,u is actually a unary operation on v, because u is derived from v.
u = f(v)
x = g(v,u)| return !isempty(collected_incident(d, var, [:src, :proj1, :proj2, :summand])) | ||
| end | ||
|
|
||
| function get_variable_parents(d::SummationDecapode, var::Int) |
There was a problem hiding this comment.
can we rename all these java getters to remove get_
|
|
||
| function collected_incident(d::ACSet, searches::AbstractVector, args...) | ||
|
|
||
| isempty(searches) && error("Cannot have an empty search") |
There was a problem hiding this comment.
shouldn't empty search return empty list?
|
|
||
| export collected_incident | ||
|
|
||
| function collected_incident(d::ACSet, searches::AbstractVector, args...) |
There was a problem hiding this comment.
docstring on exported function
|
|
||
| function collected_incident(d::ACSet, search, lookup_array) | ||
| numof_channels = length(lookup_array) | ||
| empty_outputchannels = fill(nothing, numof_channels) |
There was a problem hiding this comment.
isn't this an array of nothings? You can't push something into these "channels"
| end | ||
|
|
||
|
|
||
| function collected_incident(d::ACSet, search, lookup_array, output_array) |
There was a problem hiding this comment.
What is the type of search?
|
|
||
| function collected_incident(d::ACSet, search, lookup_array, output_array) | ||
| length(lookup_array) == length(output_array) || error("Input and output channels are different lengths") | ||
| isempty(lookup_array) && error("Cannot have an empty lookup") |
There was a problem hiding this comment.
should empty lookup_array return an empty list? empty search => empty results makes sense to me.
| index_result = incident(d, search, lookup) | ||
| isnothing(output_channel) ? index_result : d[index_result, output_channel] | ||
| end | ||
|
|
There was a problem hiding this comment.
this file feels over engineered for something simple, but I can't tell what it is supposed to do because of the lack of docstrings.
| sort(test) == sort(expected) | ||
| end | ||
|
|
||
| get_index_from_name(d::SummationDecapode, varname::Symbol) = only(incident(d, varname, :name)) |
There was a problem hiding this comment.
no java please. just call this index rely on dispatch to use the name because you gave it a symbol.
| using ACSets | ||
|
|
||
| function array_contains_same(test, expected) | ||
| sort(test) == sort(expected) |
There was a problem hiding this comment.
so you want to handle multiplicity? Could replace with setequal(a,b) = Set(a) == Set(b)
| end | ||
|
|
||
| function get_variable_parents(d::SummationDecapode, var::Int) | ||
| return collected_incident(d, var, [:tgt, :res, :res, [:summation, :sum]], [:src, :proj1, :proj2, :summand]) |
There was a problem hiding this comment.
I have no idea what this is supposed to do, is this supposed to be a conjucnction of disjunctions type query?
This PR adds a higher level
collected_incidentfunction that is meant to serve as a replacement to the many reduce-vcats we tend to use when trying to call incident on multiple things at once. Since results from this function are always combined into a single vector of unique results, as opposed toincidentwhich may return a vector of vector of results, care must be taken when calling this function into different tables or when calling with multiple inputs.This function is meant to be generic to work well with the idea that DiagrammaticEquations should have abstracted algorithms but I've also added specific helper functions for Decapodes that may be useful.