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

multicolumn indexing #74

Closed
wants to merge 35 commits into from
Closed

Conversation

slwu89
Copy link
Member

@slwu89 slwu89 commented Oct 28, 2023

I'm opening a draft PR for features to address #17. There's some junk/temporary files that I was using to test things with that are in this PR, but the relevant "proof of concept" code is in https://github.com/slwu89/ACSets.jl/blob/multicol-index/src/MultiColIndex.jl and https://github.com/slwu89/ACSets.jl/blob/multicol-index/test/MultiColIndex.jl contains some bare minimum examples that things are working with one indexed product, and one an indexed multi-relation.

On the dev meeting, it was suggested by @epatters that a good way to incorporate the new data structures would be to wrap the acset in another structure that contained information needed for indexing. This draft creates a new type IndexedSpanACSet which stores what I am calling "indexed spans", that is, (multi)spans that we want to generate a preimage cache for, and also stores those preimage caches. The indexed spans are stored in the type, as eventually I think we'll want to move as much of the search logic as possible to be done at compile time, but for now all of that logic is done at runtime.

incident is overloaded for the new type, and either looks up the key in the cache if that combination of homs is indexed, or does a simple runtime search if not.

In order to nicely integrate this feature with existing ACSet machinery, it seems that modifications to at least 4 methods are needed. If parts are added or removed to the legs of any indexed span, the keys of the preimage caches need to be updated, so add_part! and rem_part! are overloaded. On a second glance at the source, it seems that add_part! actually just calls add_parts! with a single argument, so it seems that the overload should be for the plural method. The other two are set_subpart! or clear_subpart!, which need to update values associated with keys when subparts are modified that are involved in an indexed span(s). I believe that overloading these 4 methods, and forwarding the rest of the methods in the acset interface to the underlying acset within the IndexedSpanACSet should be sufficient to get everything working, but this is one of many places I'm looking for some guidance.

There are many remaining questions besides obvious ones related to eventually getting this PR ready for review. One of them is how (assuming we want to) to allow multicolumn indexing to work where some of the parts in incident may be attributes, and we want to index backwards along some chain of attrs/homs until we get to the apex part(s) corresponding to them.

Tagging @kris-brown here as he gave me great help on my last PR, but happy to hear comments from others.

@codecov
Copy link

codecov bot commented Oct 28, 2023

Codecov Report

Attention: 81 lines in your changes are missing coverage. Please review.

Comparison is base (8ffb2d8) 91.70% compared to head (40f7022) 86.62%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #74      +/-   ##
==========================================
- Coverage   91.70%   86.62%   -5.08%     
==========================================
  Files          17       18       +1     
  Lines        1362     1443      +81     
==========================================
+ Hits         1249     1250       +1     
- Misses        113      193      +80     
Files Coverage Δ
src/ACSets.jl 100.00% <ø> (ø)
src/MultiColIndex.jl 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@slwu89
Copy link
Member Author

slwu89 commented Jan 13, 2024

Closing, as I haven't had time to keep pursuing it, and AlgebraicJulia/GATlab.jl#135 seems like the glorious future for acset like data structures.

@slwu89 slwu89 closed this Jan 13, 2024
@slwu89 slwu89 mentioned this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant