NetworkLayout.jl icon indicating copy to clipboard operation
NetworkLayout.jl copied to clipboard

adding fixed locations to SFDP - update

Open claytonpbarrows opened this issue 1 year ago • 9 comments

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

claytonpbarrows avatar Aug 19 '22 20:08 claytonpbarrows

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.

thchr avatar Aug 25 '22 22:08 thchr

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?

claytonpbarrows avatar Aug 26 '22 11:08 claytonpbarrows

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 avatar Aug 26 '22 15:08 thchr

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...

thchr avatar Aug 26 '22 15:08 thchr

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

hexaeder avatar Aug 29 '22 06:08 hexaeder

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

claytonpbarrows avatar Sep 02 '22 11:09 claytonpbarrows

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...

hexaeder avatar Sep 02 '22 14:09 hexaeder

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.

claytonpbarrows avatar Sep 03 '22 03:09 claytonpbarrows

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%) :arrow_down:
src/spring.jl 96.00% <0.00%> (-1.78%) :arrow_down:
src/stress.jl 96.93% <0.00%> (-0.82%) :arrow_down:
src/shell.jl 100.00% <0.00%> (ø)
src/squaregrid.jl 100.00% <0.00%> (ø)
src/buchheim.jl 97.32% <0.00%> (+0.13%) :arrow_up:
src/spectral.jl 96.15% <0.00%> (+0.15%) :arrow_up:
src/NetworkLayout.jl 94.73% <0.00%> (+0.29%) :arrow_up:

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

codecov[bot] avatar Sep 03 '22 14:09 codecov[bot]