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

[WIP] crossing utilities and functions #269

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

narnold0
Copy link

@narnold0 narnold0 commented Jul 2, 2023

I use the words "layout" and "positions" interchangeably sometimes, but I just mean the node positions at this point.

-Added functions to take a Graph and vector of nv positions, and calculate crossing points.

  1. Currently just straight line segments, but do wanna extend it for curves, arcs, and such

  2. multiple edges crossing at the same point should purposefully lead to one unique point counted as their crossing point, but the crossings between the edges as pairs are still counted also in matrix representation of the crossings. In other words, three edges which cross at the same point will have one crossing point associated with them, but all of the pairs are counted.

  3. It uses a mapping 'crossings' with a pair of edges as a key and their crossing point as the value, so multiple edges crossing at the same point should get mapped to the same value. So one can differentiate the number of edge pairs that intersect, and also the number of actual separate crossing points as stated above.

  4. It should work with multiedges almost as is, but visually it is weird as since it only uses the node positions and line segments. So with digraphs, for example, with a pair of vertices that have an out and also in edge between them, the crossing points are between the two edges.

-Added a sparse E x E matrix to represent crossings, where for undirected graphs it is symmetric and an entry denotes a crossing between ei and ej. For directed, something like the entry in row ei , col ej being +1 if from the perspective of traveling on ej, the crossing goes through its right side, and -1 if ei goes through its left side.

-Added function to create a new graph from crossings of another, where the is a vertex for each crossing and two are connected by an edge if their "crossings" share a common edge in the original.

-Added a function to create a different new graph from crossings, where there is a vertex in the new graph for every edge with at least one crossing, and two vertexes are joined if they have crossings together in the original graph. The sparse matrix mentioned earlier with the empty row/cols removed is the adj matrix of this graph.

-Added a function, which I don't think works currently, for the "natural" planarized plane graph of another graph with crossings, where crossings become vertices and are connected as one would expect

-Added functions for the crossings associated with an edge, or a vertex.

-Added a function crossinginfo to take a (G, positions) pair and create/return a dictionary mapping to associate them. First using G as a key, and then using the positions as a dict key within a nested dict to return a third, in which info on the pair was put. Pretty much G => positions => Dictionary with calculated properties of the pair
The idea being one could pass solely the graph, and then receive the first-level dictionary, and be able to cycle the different embedding properties by passing its respective layout as a key.

Currently, from my understanding, and from asking in meshes, there is no real (non-dilapidated) Julia implementation of a Bentley–Ottmann type algorithm for crossings of an entire set of segments, and the ability to return the actual points, and not just a truth value of the possibility of an intersection.
I looked at other packages like LazySets and CombinatorialSpaces also. With the latter, I hadn't encountered what it deals with previously, but it seems to be very much interrelated to this type of thing with graphs?

So at the moment it just does the worst case by checking all edge pairs for an intersection with meshes. I've tried versions using python libraries that implement this algorithm and whether it was how I using them, or python or what, I just couldn't get them to be faster, idk. This is my first time using GitHub also, so apologies for anything that isn't how it should be, was just a challenge to get this here.

@narnold0 narnold0 changed the title crossing utilities and functions [WIP] crossing utilities and functions Jul 2, 2023
@codecov
Copy link

codecov bot commented Jul 2, 2023

Codecov Report

Merging #269 (948074e) into master (3c97457) will decrease coverage by 2.81%.
Report is 29 commits behind head on master.
The diff coverage is 0.00%.

❗ Current head 948074e differs from pull request most recent head 6461a14. Consider uploading reports for the commit 6461a14 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #269      +/-   ##
==========================================
- Coverage   97.17%   94.36%   -2.81%     
==========================================
  Files         114      115       +1     
  Lines        6656     6855     +199     
==========================================
+ Hits         6468     6469       +1     
- Misses        188      386     +198     

@gdalle gdalle linked an issue Jul 2, 2023 that may be closed by this pull request
@gdalle gdalle added the enhancement New feature or request label Jul 2, 2023
function and logic for conveying direction of crossing with directed edges
@gdalle
Copy link
Member

gdalle commented Jul 5, 2023

Thanks for the contribution! Instead of giving a commented example usage, it would be better to add tests to the package test suite, do you know how to do that?

@narnold0
Copy link
Author

narnold0 commented Jul 5, 2023

Thanks for the contribution! Instead of giving a commented example usage, it would be better to add tests to the package test suite, do you know how to do that?

Not at all if I'm being honest. It's quite confusing online to find what I'm supposed to do

@gdalle
Copy link
Member

gdalle commented Jul 5, 2023

It's okay, I'll take you through it 😊 I'm a bit busy this week but I'll try to show you next week

@narnold0
Copy link
Author

narnold0 commented Sep 21, 2023

Are there any good resources or anything online that anybody could recommend for implementing tests and passing the "codecov/patch" check successfully?

I have been occupied up until now with a lot and I haven't updated this. I have some more things I want to add, but first I want to ensure what is here works before trying to add more.

Many thanks for any guidance on this

@gdalle
Copy link
Member

gdalle commented Sep 29, 2023

Hi, sorry for the delay in answering

The best thing to do is to try and imitate other tests. Look for an algorithm that does something similar to yours, then scour the test folder to see where it is tested, and try to replicate it.

Code coverage denotes the fraction of the code that is covered by tests, i.e. the percentage of lines of the source that get executed when you run the tests. If your new functionality is properly tested, this should pass automatically

@narnold0
Copy link
Author

narnold0 commented Jan 20, 2024

Have written a good number of the tests. Might be some that I must still do. I still need to find a sufficient library for curves. Would then allow arbitrary multigraphs, self-intersections, and multiple crossings of the same edges, etc... Not sure about the options for curve libraries in that regard. Also will add a function for the number of connected regions in a drawing of a graph.

Edit: Seems the codecov bot updated, but I added 20 tests that are successful and it doesn't seem like it is showing them as coverage. I don't think it's counting what I added or something

@narnold0
Copy link
Author

narnold0 commented Jan 24, 2024

Also, this is bottlenecked by the naive $n^2$ pairwise crossing detection. I've looked at every Benttley-Ottman Alg. implementation I can find online and they either don't have the needed interface (ie one cant receive edges corresponding to a crossing event) or they aren't any faster. Several of the implementations even say that they are eventually beaten by naive checking, and with every one I've implemented this has been true more or less.

My understanding is that if a suitable implementation of BO was available, then implementations of the Clarkson, Cole & Tarjan (1992) and Eppstein, Goodrich & Strash (2009) would allow linear crossing detection and planarization for almost all graphs.

@narnold0 narnold0 closed this Jan 24, 2024
@narnold0 narnold0 reopened this Jan 24, 2024
@gdalle
Copy link
Member

gdalle commented Jan 29, 2024

Hi,
Sorry for the delay. I had not realized how much of the code depended on external libraries for plotting. As a result I would say it does not belong in Graphs.jl. Maybe in GraphMakie.jl?

@narnold0
Copy link
Author

narnold0 commented Jan 29, 2024

Hi, Sorry for the delay. I had not realized how much of the code depended on external libraries for plotting. As a result I would say it does not belong in Graphs.jl. Maybe in GraphMakie.jl?

The plotting isn't actually necessary and have also used plots.jl etc... The graph functions are not dependent on visual plotting and all the visual plotting can be removed without affecting any of the functions, and it is obviously faster that way, I just use them for clarity and brevity. Sorry if I have them in there still

The actual base inputs are a graph, and (for now) the corresponding (x,y) positions of the vertices in the correct format. The latter is just a geometrybasics tuple atm if I'm remembering right

@gdalle
Copy link
Member

gdalle commented Jan 30, 2024

Still there are other dependencies like Meshes, Combinatorics or GeometryBasics which I don't think should be required by Graphs.jl.
NetworkLayout.jl on the other hand might be a more suitable place, and it already depends on GeometryBasics

@narnold0
Copy link
Author

narnold0 commented Jan 30, 2024

Still there are other dependencies like Meshes, Combinatorics or GeometryBasics which I don't think should be required by Graphs.jl. NetworkLayout.jl on the other hand might be a more suitable place, and it already depends on GeometryBasics

Understood. I must have thought those were already used in something. Would it be sufficient to remove all these extra dependencies? To replace those parts with vanilla Julia?

I think that I was trying to use their types mostly, they had "line" and "point" types so I got lazy and just used those to hold the position tuples and necessary information. I Could make these regular julia nested tuples, or maybe a custom struct in Julia? which of these is "better" in Julia?

@gdalle
Copy link
Member

gdalle commented Feb 2, 2024

It's a difficult question. If the structures exist within another package, it would be a shame not to use them. At the same time, if you import a full other package just to use a tiny percentage of its functionalities, maybe another approach would make more sense.

Each additional dependency induces additional compilation time, which is why I'm very reluctant to add any to Graphs.jl. On the other hand, https://github.com/JuliaGraphs/NetworkLayout.jl is less widely used and already depends on GeometryBasics.jl + StaticArrays.jl (very good for storing $2$-vectors), so I think your contribution would be a better fit there.

Fair warning: I have never touched that package so even though I am a JuliaGraphs maintainer, I won't necessarily be the best fit for review

@narnold0
Copy link
Author

narnold0 commented Feb 27, 2024

An update. I've removed Meshes and Combinatorics so far, with only GeometryBasics left to do. At the same time, most of the important code has been rewritten for those functionalities that were used, and it is substantially faster, and this will hopefully only increase.

But, given the nature of the crossing detection, I think that parallel processing is needed at a certain size for it to be high performance, so I've been starting to try to learn CUDA to that effect. (Also AMD ROCm, but I only have a Nvidia card atm).

I had read that in 1.9, Julia now supports the ability for optional weak dependecies and extensions` through this.

Do you think such GPU parallel processing libraries might be possible as a weak dependency for Graphs.jl?

@simonschoelly
Copy link
Contributor

An update. I've removed Meshes and Combinatorics so far, with only GeometryBasics left to do. At the same time, most of the important code has been rewritten for those functionalities that were used, and it is substantially faster, and this will hopefully only increase.

But, given the nature of the crossing detection, I think that parallel processing is needed at a certain size for it to be high performance, so I've been starting to try to learn CUDA to that effect. (Also AMD ROCm, but I only have a Nvidia card atm).

I had read that in 1.9, Julia now supports the ability for optional weak dependecies and extensions` through this.

Do you think such GPU parallel processing libraries might be possible as a weak dependency for Graphs.jl?

We might think about this in the future, but for now we try to still support v1.6. - I am a bit skeptical about CUDA even as a week dependency - it might pull in a lot of other unnecessary dependencies and will only work on machines with an Nvidia GPU - and from my experience, at least on linux, CUDA drivers will break from time to time.

So my suggestion is, that you first develop this without the use of CUDA. Then you can create a separate version of an algorithm that uses CUDA, but add it to an external package while resusing some of the things that you set up in Graphs.jl (ex. GraphsCUDA.jl, GraphsGPU.jl - or maybe even the proposed GraphsExtras.jl) We will be more than happy to add such a package to the JuliaGraphs org - and in the future we might also create a unified documentation.

@gdalle
Copy link
Member

gdalle commented Mar 5, 2024

I agree with avoiding CUDA, but even theme-wise I think those algorithms belong in NetworkLayout?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Would graph crossing utilities belong here?
3 participants