Skip to content

adding fixed locations to SFDP - update #52

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 5 commits into from

Conversation

claytonpbarrows
Copy link

this is a redo on #29 based on v0.4

@thchr
Copy link
Collaborator

thchr commented Aug 25, 2022

I would love to use this!

I wonder though if the interface could be designed to allow greater flexibility in what to "fix": e.g., it might be nice to be able to fix just a single coordinate of a node (i.e. x or y coordinate) in addition to specifying both coordinates simultaneously.

Tentatively, could the interface be a Vector or Dict of Vector{Union{Nothing, <:Real}} instead?
EDIT: I think aninterface where the "pinning"/"fixing" is done via a variable pin :: Union{Nothing, Vector{Point{Dim,Bool}}} could work well, with pinning of the jth coordinate of the ith position decided by the value of pin[i][j]. To disable any pinning whatsoever, one could set pin to nothing.

@claytonpbarrows
Copy link
Author

In the current implementation, and in the suggested change where pinning is done via a variable pin :: Union{Nothing, Vector{Point{Dim,Bool}}}, the adjacency matrix must be sorted so that the n edges with n known positions occur first. That is consistent with the initialpos implementation. Is that what you're imagining here?

@thchr
Copy link
Collaborator

thchr commented Aug 26, 2022

I was thinking that the vertex iterator of the graph and pin should just have identical lengths. It's true that it might be nice to allow pin to potentially just have the length of initialpos though.

Without the initialpos-length complication, we could do this by adding a pin::Union{Nothing, Vector{Point{Dim,Bool}}} field to SFDP and then changing the location update to:

norm_force = norm(force)
if !isnothing(algo.pin)
    locs[i] += (!).(algo.pin[i]) .* (force * (step/norm_force))
else
    locs[i] += force * (step/norm_force)
end

If we allow pin's length to be shorter than the number of vertices (but equal to initialpos), then we'd need a length check like what you currently have as well.

@thchr
Copy link
Collaborator

thchr commented Aug 26, 2022

Tangentially, it would be nice to add this functionality to Spring as well (which has a very similar update step).
Might be that Stress could have this too, but the update step is a bit different...

@hexaeder
Copy link
Collaborator

hexaeder commented Aug 29, 2022

I'd also really prefere to find a nice interface which could be added to all the interative layouts. I am not plotting that many graphs nowadays but when i wanted to use this feature I was allways thinking of an dict based interface. Something like
pin = Dict{Int, Point}(2 => (1,0), 7=>(-1,-1) which could be used to initialise the position and tell the algorithm wich vertices to pin at the same time

@claytonpbarrows
Copy link
Author

@hexaeder I like your interface improvement suggestion. I'd suggest that the same interface should be used for initialpos and pin. Do you agree?

@hexaeder
Copy link
Collaborator

hexaeder commented Sep 2, 2022

I think there is an important usage difference between initialpos and pin: If you want to give a special initialisation for the algorithm you're likely to do so for the whole graph, then the vector makes sense [1].
At least for my usage pin is different in that regard: normally I want to place a few key vertices at some positions and the rest will should group arround them according to the algorithm.

However, as long as it is non-breaking it would be cool if initialpos could handle both? Something like

struct Spdf{..., IPT} where {IPT <: Union{Dict, Vector}}
     initialpos::IPT
end

# and during construction
startpos = rand(N)
for (k,v) in pairs(algo.initialpos)
    startpos[k] = v
end

[1] The fact that you may specify the first M initial positions of a graph with N != M vertices is a more or less unintended side effect of the interface. For GraphMakie i wanted to fix layout parameters before actual applying the layout to a specific graph. To increase flexibility I chose it in a way, that you can specify 10 initial positions and the algorithm will still produce layouts for graphs with 9 or 11 vertices...

@claytonpbarrows
Copy link
Author

This latest attempt gets most of the way towards the suggestions from @hexaeder
To summarize:

  • initialpos can handle:
    • Vector{Tuple} - in this case it creates initial positions for the N=length(initialpos) nodes
    • Vector{Point} - same as above
    • Dict{Int, Point} - provides more control since you can specify the initial position of any subset of nodes
  • pin can handle
    • Vector{Bool} - pins the initial position of nodes with the same indices of true values. If true entries have indices that don't match initialpos a warning is thown. Otherwise there are no requirements about the length of the vector
    • Dict{Int, Bool} - same but uses dictionary indexing

I don't think this quite meets all the requests. And it's not the cleanest implementation, but I've spent too much time on this now and I have to put it down.

@codecov
Copy link

codecov bot commented Sep 3, 2022

Codecov Report

Merging #52 (43f732a) into master (8eca84d) will decrease coverage by 0.67%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master      #52      +/-   ##
==========================================
- Coverage   97.44%   96.76%   -0.68%     
==========================================
  Files           8        8              
  Lines         470      526      +56     
==========================================
+ Hits          458      509      +51     
- Misses         12       17       +5     
Impacted Files Coverage Δ
src/sfdp.jl 95.40% <88.88%> (-2.91%) ⬇️
src/spring.jl 96.00% <0.00%> (-1.78%) ⬇️
src/stress.jl 96.93% <0.00%> (-0.82%) ⬇️
src/shell.jl 100.00% <0.00%> (ø)
src/squaregrid.jl 100.00% <0.00%> (ø)
src/buchheim.jl 97.32% <0.00%> (+0.13%) ⬆️
src/spectral.jl 96.15% <0.00%> (+0.15%) ⬆️
src/NetworkLayout.jl 94.73% <0.00%> (+0.29%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@hexaeder
Copy link
Collaborator

closed by #53

@hexaeder hexaeder closed this Mar 20, 2023
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