-
Notifications
You must be signed in to change notification settings - Fork 47
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
Add AmalgamDigraphs
#532
Open
finnbuck
wants to merge
11
commits into
digraphs:main
Choose a base branch
from
finnbuck:amalgamDigraphs
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Add AmalgamDigraphs
#532
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
finnbuck
force-pushed
the
amalgamDigraphs
branch
from
March 17, 2022 16:32
b853de6
to
dfb2612
Compare
finnbuck
force-pushed
the
amalgamDigraphs
branch
from
March 17, 2022 16:45
dfb2612
to
1f46844
Compare
finnbuck
force-pushed
the
amalgamDigraphs
branch
from
March 30, 2022 15:18
3f90318
to
3110b0c
Compare
finnbuck
force-pushed
the
amalgamDigraphs
branch
from
March 30, 2022 15:36
3110b0c
to
5fae321
Compare
james-d-mitchell
requested changes
Mar 31, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@finnbuck looks great! Lots of small things to fix, but generally this is good! Some additional things:
- please also add some tests when the arguments are mutable digraphs, I don't think this'll work quite like you expect (see comments in the PR)
- Add a version that takes 4 digraphs,
D1
,D2
, andS
and a transformationmap1
(embeddingS
inD1
), then computesmap2
(an embedding ofS
inD2
) and callsDigraphAmalgam(D1, D2, S, map1, map2);
- Add a version that takes 3 digraphs:
D1
,D2
andS
and computesmap1
andmap2
(embeddings intoD1
andD2
resp), and callsDigraphAmalgam(D1, D2, S, map1, map2);
- Add a version that takes 2 digraphs
D1
andD2
, and callsx := MaximumCommonSubdigraph(D1, D2);
and then returnsDigraphAmalgam(D1, D2, x[1], x[2], x[3]);
@james-d-mitchell thanks for the review, I'll get to work on that! |
Co-authored-by: James Mitchell <[email protected]>
Co-authored-by: James Mitchell <[email protected]>
finnbuck
force-pushed
the
amalgamDigraphs
branch
2 times, most recently
from
April 13, 2022 15:20
a337700
to
a604293
Compare
finnbuck
force-pushed
the
amalgamDigraphs
branch
from
April 13, 2022 15:28
a604293
to
49e5ef5
Compare
finnbuck
changed the title
Initial version of AmalgamDigraphs and AmalgamDigraphsIsomorphic.
AmalgamDigraphs
May 11, 2022
finnbuck
force-pushed
the
amalgamDigraphs
branch
2 times, most recently
from
May 11, 2022 07:17
f10e594
to
2528c7b
Compare
finnbuck
force-pushed
the
amalgamDigraphs
branch
6 times, most recently
from
May 11, 2022 08:21
f711d61
to
3f5ce71
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on Issue #428. This pull request adds two functions: AmalgamDigraphs and AmalgamDigraphsIsomorphic.
Description:
The AmalgamDigraphs function takes as input two digraphs and two lists of vertices corresponding to two identical subdigraphs of the two input digraphs. It returns a new digraph which consists of the two input digraphs joined together by their common subdigraph in such a way that the edge connectivity between the vertices in the new digraph matches the edge connectivity of the two input digraphs.
AmalgamDigraphsIsomorphic functions similarly, but the two lists of vertices given as inputs need only describe subdigraphs that are isomorphic to one another rather than have them be precisely equal. AmalgamDigraphsIsomorphic then rearranges the second list of subdigraph vertices so that the induced subdigraphs become equal and then calls AmalgamDigraphs.
Both functions return a tuple of size two, with the first element being the output digraph and the second being a record which maps each vertex number in the second input digraph to its new vertex number in the output digraph. The mapping of the vertices of the first input digraph can be seen as the identity mapping.