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

Decorated graphs #37

Merged
merged 3 commits into from
Jul 18, 2023
Merged

Decorated graphs #37

merged 3 commits into from
Jul 18, 2023

Conversation

JoeyT1994
Copy link
Contributor

This PR, which addresses #36, adds functionality for decorating graphs by adding additional graphs on their edges or vertices.

Two functions are provided

decorate_graph_edges(g::NamedGraph; edge_map::Function=e -> named_grid((1,)))

and

decorate_graph_vertices(g::NamedGraph; vertex_map::Function=v -> named_grid((1,)))

Currently there are defaults for which vertices of the decoration are connected to the edges/vertices original graph but we will add keyword arguments for this in the future.

Testing is included. This PR also fixed an ambiguity issue with function a_star when passed a NamedGraph with Integer labelling.

@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2023

Codecov Report

Merging #37 (fbcca29) into main (2a0217b) will increase coverage by 0.63%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main      #37      +/-   ##
==========================================
+ Coverage   80.29%   80.92%   +0.62%     
==========================================
  Files          20       21       +1     
  Lines         878      907      +29     
==========================================
+ Hits          705      734      +29     
  Misses        173      173              
Impacted Files Coverage Δ
src/NamedGraphs.jl 100.00% <ø> (ø)
src/Graphs/generators/decoratedgraphs.jl 100.00% <100.00%> (ø)
src/abstractnamedgraph.jl 83.83% <100.00%> (+0.24%) ⬆️

@mtfishman
Copy link
Member

For the current version of vertex_map, aren't the edges connecting to the decorated vertex essentially random since the ordering that neighbors outputs is an internal detail? Seems like until we decide how to specify those new edges, it's not so useful.

@mtfishman
Copy link
Member

Maybe vertex_map could be a function that accepts a list of vertices, where the first one is the vertex and the rest are the neighbors of that vertex. Then the user is expected to add edges between their new vertices in the vertex decoration and the neighbor vertices. If those aren't added, then those edges could be added (essentially) randomly as you do it now.

@JoeyT1994
Copy link
Contributor Author

JoeyT1994 commented Jul 14, 2023

For the current version of vertex_map, aren't the edges connecting to the decorated vertex essentially random since the ordering that neighbors outputs is an internal detail? Seems like until we decide how to specify those new edges, it's not so useful.

Currently the vertex vd which is being decorated is removed and replaced with the decoration graph gd. The edges which originally connected vd to its neighbors in the undecorated graph vd_neighbors are reconnected to the first vertex of gd? So there is nothing random going on but yeah the user should ideally decide which vertices of gd should connect up to which members of vd_neighbors ?

@mtfishman
Copy link
Member

mtfishman commented Jul 14, 2023

Oh I see, I was picturing that each vertex neighbor would get connected to unique vertices of the decorated graph getting inserted onto the vertex, that's why I thought it would be random to some extent. In that case, I agree it isn't random (though still somewhat arbitrary and restricted) since the user can choose which is the first vertex of the decorated vertex graph. In that case it seems fine to leave it for future work to give more control to the users over how the edges to the neighbors get specified.

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.

3 participants