-
Notifications
You must be signed in to change notification settings - Fork 93
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
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 |
function and logic for conveying direction of crossing with directed edges
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 |
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 |
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 |
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 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 |
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 |
Also, this is bottlenecked by the naive 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. |
Hi, |
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 |
Still there are other dependencies like |
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? |
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 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 |
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 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. |
I agree with avoiding CUDA, but even theme-wise I think those algorithms belong in NetworkLayout? |
Hi yall, Has been a minute for sure. I've mainly been focused on other things for the most part as I'm in my senior year. |
Hi @narnold0, |
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.
Currently just straight line segments, but do wanna extend it for curves, arcs, and such
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.
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.
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 muchG => 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.