Skip to content

adding fixed locations to sfdp #29

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

Closed
wants to merge 1 commit into from

Conversation

claytonpbarrows
Copy link

I commonly use SFDP to determine the locations of a subset of vertices when some locations are known. This PR implements this functionality by adding the fixed kwarg.
If this is too niche, please let me know and I'll close the PR.

@SimonDanisch SimonDanisch requested a review from hexaeder June 1, 2021 20:51
@codecov
Copy link

codecov bot commented Jun 1, 2021

Codecov Report

Merging #29 (8c45251) into master (00491f9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #29   +/-   ##
=======================================
  Coverage   98.62%   98.62%           
=======================================
  Files           7        7           
  Lines         363      364    +1     
=======================================
+ Hits          358      359    +1     
  Misses          5        5           
Impacted Files Coverage Δ
src/sfdp.jl 98.11% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00491f9...8c45251. Read the comment docs.

@hexaeder
Copy link
Collaborator

hexaeder commented Jun 1, 2021

I don't think this is too niche, fixed positions for Spring and SFDP layouts are definitely on my feature wishlist for the next release. However I've recently started quite a big refactor of this package in #27 (wip docs), I think we should finish this process first before adding new features. Comments on this new interfaces are allways welcome!

The main idea is to split the parameters of each algorithm from it's actual application to a graph. The main problem here is to keep the algorithm parametrization as general as possible, for example, it should be possible to add more nodes and edges to the graph without need of reparametrizing the algorithm. I'd therefore suggest to implement this as an Dict{Int, Point{dim, Ptype}} instead of a BitVector and initial positions of fixed size.

layout = SFDP(; fixed=Dict(1=>Point(0,0), 3=>Point(1,1))
positions = layout(adj_matrix)

Would still work with graphs of different sizes by filling up all of the other field with random positions. This comes quite handy if you don't wan't to think about the initialization of all of the other start positions.
I becomes quite unelegant in the presence of both (initial positions and fixed positions) though, where the fixed positions should take precedence....

@claytonpbarrows
Copy link
Author

OK, once you have merged #27, I'll rebase and address your comment.

@hexaeder
Copy link
Collaborator

hexaeder commented Jul 1, 2021

just wanted to bump this because 0.4 is released now

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.

2 participants