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

New package: NahaGraphs v0.1.0 #96626

Conversation

JuliaRegistrator
Copy link
Contributor

@JuliaRegistrator JuliaRegistrator commented Dec 6, 2023

This is the first version of NahaGraphs to be registered.

NahaGraphs provides a simple representation for directed graphs and an interface for rendering them using GraphViz.

This implementation is simple-mined and will be inefficient for large graphs.

One use for NahaGraphs is to construct a miror of your application's tree of immutable structs in order to be able to follow reverse edges.

Copy link
Contributor

github-actions bot commented Dec 6, 2023

Your new package pull request does not meet the guidelines for auto-merging. Please make sure that you have read the General registry README and the AutoMerge guidelines. The following guidelines were not met:

  • Package name similar to 1 existing package.
    1. Similar to DataGraphs. Damerau-Levenshtein distance 2 is at or below cutoff of 2.

Note that the guidelines are only required for the pull request to be merged automatically. However, it is strongly recommended to follow them, since otherwise the pull request needs to be manually reviewed and merged by a human.

After you have fixed the AutoMerge issues, simple retrigger Registrator, which will automatically update this pull request. You do not need to change the version number in your Project.toml file (unless of course the AutoMerge issue is that you skipped a version number, in which case you should change the version number).

If you do not want to fix the AutoMerge issues, please post a comment explaining why you would like this pull request to be manually merged. Then, send a message to the #pkg-registration channel in the Julia Slack to ask for help. Include a link to this pull request.

Since you are registering a new package, please make sure that you have also read the package naming guidelines: https://julialang.github.io/Pkg.jl/dev/creating-packages/#Package-naming-guidelines-1


If you want to prevent this pull request from being auto-merged, simply leave a comment. If you want to post a comment without blocking auto-merging, you must include the text [noblock] in your comment. You can edit blocking comments, adding [noblock] to them in order to unblock auto-merging.

UUID: fd225a7d-cc45-47cf-8742-8ffafb057088
Repo: https://github.com/MarkNahabedian/NahaGraphs.jl.git
Tree: 5e6fac6731a77b605bbedf634d8c78ade5a0dc10

Registrator tree SHA: 17aec322677d9b81cdd6b9b9236b09a3f1374c6a
@JuliaRegistrator JuliaRegistrator force-pushed the registrator-nahagraphs-fd225a7d-v0.1.0-a6b2af7983 branch from b9a16fa to 97eff1f Compare December 6, 2023 16:25
@gdalle
Copy link
Contributor

gdalle commented Dec 6, 2023

Hi, and congrats on the new package! There are already plenty of options for graph formats, can I maybe ask why you needed to roll out a new one? Also the fact that the package seems named after yourself makes me doubt whether it's intended for wider use.

@MarkNahabedian
Copy link

@gdalle: The reason I'm considering registering NahaGraphs.jl is that I'm using it in PanelCutting.jl, which I would eventually like to register.

I'm looking at MetaGraphs to see if it will meet that use case. It's a bit annoying in that I need to maintain a mapping between that application's objects, which the graph nodes represent, and integers, which seem to be the only things that Graphs.jl and its friends understand.

There would still be the question of where to put the code that, given a graph, writes a GraphViz dot file. Maybe I can get it added to https://github.com/JuliaGraphs/GraphViz.jl. I expect the maintainers of that package might welcome code to write dot files, but might not want to support another package's graph representation when that package has its own, which, sadly, is not documented. Lookiing at the source code, this appears to just be a vaneer over a C library. I wich they outright said that.

Until I either resolve these or give up on them, registration of NahaGraphs.jl should be blocked.

@MarkNahabedian
Copy link

I've looked at replacing PanelCutting's use of NahaGraphs with MetaGraphs and Graphs.

Neither Graphs nor MetaGraphs provide an efficient way to map between objects of the application and nodes of the graph. I don't understand how those packages can be at all useful without such a mechanism.

I can wrap a MetaGraph with a struct that provides a Dict which maps from application objects to graph node numbers. I can't imagine ever using Graohs or MetaGraphs without such a wrapper. Thus, I'd need to put that wrapper and the related code into a package so that it's available where needed. That package might as well be NahaGraphs or whatever I rename it to.

@gdalle
Copy link
Contributor

gdalle commented Dec 11, 2023

Neither Graphs nor MetaGraphs provide an efficient way to map between objects of the application and nodes of the graph. I don't understand how those packages can be at all useful without such a mechanism.

Actually, both MetaGraphs.jl and its successor MetaGraphsNext.jl do provide this functionality.

  • In MetaGraphs.jl, it is done with an "indexing property", declared as follows: set_indexing_prop!(G, :name)
  • In MetaGraphsNext.jl, it is done with "vertex labels" (not to be confused with "vertex codes" which are the integers). See the documentation for examples.

Does that answer your needs?

@MarkNahabedian
Copy link

Thanks for your help.

I find the documentation in the form of examples unrelated to my needs to be useless. There's no conceptual overview. The only thing left is to take stabs in the dark using @doc.

I want to include arbitrary Julia objects as nodes in a graph. Here's a minimal example:

using Graphs
using MetaGraphs

g = MetaGraph()

set_indexing_prop!(g, :object)

let
    next_thingy_id = 1

    struct Thingy
        id
        
        function Thingy()
            t = new(next_thingy_id)
            next_thingy_id += 1
            t
        end
    end
end

t1 = Thingy()
t2 = Thingy()
t3 = Thingy()

add_vertex!(g, t1)
add_vertex!(g, t2)
add_vertex!(g, t3)

add_edge!(g, t1, t2)
add_edge!(g, t1, t3)

# Now find all of the edges of g that lead to t3.
```

The above code gets the error

```
ERROR: LoadError: MethodError: no method matching add_vertex!(::MetaGraph{Int64, Float64}, ::Thingy)
```

How do I use MetaGraphs, or whatever, in order to bdo the above?

@gdalle
Copy link
Contributor

gdalle commented Dec 12, 2023

I find the documentation in the form of examples unrelated to my needs to be useless.

While I agree that the documentation is lacking, please remember that no one is paid to take care of it, and that volunteer time is also lacking.
I can help you if you want to try and improve it. My point of view is that contributing to a flawed, but widely used, package, is often more impactful than rolling out your own.

@gdalle
Copy link
Contributor

gdalle commented Dec 12, 2023

How do I use MetaGraphs, or whatever, in order to do the above?

Here's a working example using MetaGraphsNext.jl, which is more efficient than MetaGraphs.jl.

using Graphs
using MetaGraphsNext

let
    next_thingy_id = 1

    struct Thingy
        id
        
        function Thingy()
            t = new(next_thingy_id)
            next_thingy_id += 1
            t
        end
    end
end

Base.isless(t1::Thingy, t2::Thingy) = isless(t1.id, t2.id)

g = MetaGraph(Graph(); label_type=Thingy)

t1 = Thingy()
t2 = Thingy()
t3 = Thingy()

add_vertex!(g, t1)
add_vertex!(g, t2)
add_vertex!(g, t3)

add_edge!(g, t1, t2)
add_edge!(g, t1, t3)

neighbors(g, code_for(g, t3))

@MarkNahabedian
Copy link

Thanks for the working example.

Is an isless method required for the specified label_type? In some applications it might be difficult to come up with a total ordering, even an arbitrary one. I don't think of total ordering as being intrinsic to the concept of graph
nodes.

neighbors gives me a vector of integers. How do I get to my objects from those?

code_for and neighbors seem to expose the user of the package to internal implementation details (integers) when all they want to code about are their application objects (represented by nodes in the graph) and the relationships among them (represented by edges).

I'd still need to implement methods as an abstraction barrier to those implementation details.

I would still also need code for generating a GraphViz dot file from such a graph. GraphViz_jll is poorly documented and does not appear to implement the same graph interface as other graph packages. Based on the documentation, I don't know if it will write dot files or only read them. I'm happy to either trim down (and rename) NahaGraphs to just provide the dot writing code if another graph package meets my other needs. The dot code requires that the graph expose interfaces which iterate over all nodes and all edges of the graph. In addition, it requires the application to provide methods which

  • return a unique id for a node
  • return style attributes for the graph as a whole, each node, and each edge.

I see you're a JuliaGraphs contributor. If you feel that writing GraphViz dot files fits in to that family of packages, wecan talk about integrating it there. I'd need the abstraction barrier described above first though.

I look forward to your response.

@MarkNahabedian
Copy link

Regarding the quality of documentation and the lack of volunteer time:

When one registers a package -- unleashes it to the community for broader use -- the community has a right to expect that the package will have a stated purpose which it meets, and is adequately documented and tested. If the developer doesn't have time to do this, they shouldn't register the package.

That is at least my expectation. Given what I've seen on the Julia Slack #new-packages-feed, these expectationas are rately met.

Before I submitted NahaGraphs for registration, I improved the documentation and made sure it would build and deploy properly on GitHub.

"contributing to a flawed, but widely used, package, is often more impactful than rolling out your own" and yet, there are Graphs, MetaGraphs and MetaGraphsNext.

My first impression from the Graphs documentation was "these people are interested in exploring graph algorithms on graphs that are totally abstract and have no grounding in real data or applications. I don't recall if I looked at Graphs.jl before doing my own implementation. Haviing an application in mind, this was off-putting.

I expect I searched for graph packages in julia and either only found plotting packages, or found Graphs.jl and decided it would be easier to write my own than to figure out how to use it. The very nature of my application imposed a small node and edge count, so efficiency was not a concern.

@bvdmitri
Copy link
Contributor

the package will have a stated purpose which it meets, and is adequately documented and tested

Who decides that? IMO MetaGraphsNext.jl has quite a good documentation already and it has 93% coverage.

[noblock]

@gdalle
Copy link
Contributor

gdalle commented Dec 13, 2023

About the example

Is an isless method required for the specified label_type?

Yes because of the way undirected graphs are stored by MetaGraphsNext.jl. You're right, it's not a fundamental requirement in theory, but for our current implementation it seems necessary.

neighbors gives me a vector of integers. How do I get to my objects from those?

It depends whether your "objects" are the vertex labels or the vertex data. If they are the labels, you can either use label_for after neighbors, or directly neighbor_labels.

code_for and neighbors seem to expose the user of the package to internal implementation details (integers)

Integers may be an implementation detail in theory, but in practice they are pretty central to the way Graphs.jl and its ecosystem work (for performance reasons). That means all packages for graphs with metadata had to find a way around them somehow.
There are efforts to move away from this integer requirement, but it's a huge amount of work (see the GraphsBase.jl repo)

@gdalle
Copy link
Contributor

gdalle commented Dec 13, 2023

About the documentation

When one registers a package -- unleashes it to the community for broader use -- the community has a right to expect that the package will have a stated purpose which it meets, and is adequately documented and tested. If the developer doesn't have time to do this, they shouldn't register the package.

In principle I agree. In practice, many of the packages we're talking about have been around for years, since before current standards were widely applied. Besides, their history is rather bumpy. I joined the JuliaGraphs crew later on, so me and others are trying to do our best with what we inherited, even though it's sometimes not ideal.

"contributing to a flawed, but widely used, package, is often more impactful than rolling out your own" and yet, there are Graphs, MetaGraphs and MetaGraphsNext.

Again a product of the incremental development of graph formats and ideas. It's easier to criticize with hindsight, and that's also why we're currently reflecting on other ways to do things.

My first impression from the Graphs documentation was "these people are interested in exploring graph algorithms on graphs that are totally abstract and have no grounding in real data or applications. I don't recall if I looked at Graphs.jl before doing my own implementation. Haviing an application in mind, this was off-putting.

If you take a look at the graphs ecosystem, you will find that Graphs.jl actually has a lot of applications. By nature it is indeed an abstract, foundational package, but many people have used it in many different ways.
The fact that it doesn't suit your own application perfectly doesn't mean the same applies to everyone.

I expect I searched for graph packages in julia and either only found plotting packages, or found Graphs.jl and decided it would be easier to write my own than to figure out how to use it.

That is your choice, and I won't prevent you from registering this package. I have also registered my own package for metagraphs in the past, but I regret it now. Because as I underlined on Discourse recently, there are literally tons of similar libraries with slightly different takes on what an ideal graph should be.

My goal is to reduce duplication of efforts and enhance collaboration, which is why I'm still here trying to convince you.
The upside for you would be to depend on a more widely used family of packages, where people presumably find bugs more quickly (even though they take some time to be solved).
But of course that means having to put up with code that is not exactly written the way you would have written it.

@gdalle
Copy link
Contributor

gdalle commented Dec 13, 2023

As a side note, I kindly advise you to try a softer approach when talking to open source contributors. We're all volunteers here, doing the best we can because we believe it's useful to the community.
When you barge in and say things like

I don't understand how those packages can be at all useful without such a mechanism.

I find the documentation in the form of examples unrelated to my needs to be useless. There's no conceptual overview.

My first impression from the Graphs documentation was "these people are interested in exploring graph algorithms on graphs that are totally abstract and have no grounding in real data or applications.

I must say it is slightly disheartening, and does not put people in the right mood to help you.
These packages are flawed, and I know it better than most. But instead of just criticizing them, offering to contribute even modest improvements to the docs would be very much appreciated.

@MarkNahabedian
Copy link

I'm sorry my remarks were offensive. It was not my intent to offend. I did intend to communicate my furstrations though. Rather than making sweeping generalizations, I will point out specific cases as I experience them. For example:

add_vertex! adds a vertex to a graph. It does not return that vertex. How does the caller know what that vertex is so that it could add an edge or set a property?

Fortunately MetaGraphs provides a way around this if one provides "labels".

@MarkNahabedian
Copy link

it seems I'm beiing encouraged to write documentation for a package I know very little about :-). Ok, with your help I'll bite.

I'm working on a simple example that is in line with my envisioned use cases for a graph package. I'm having a bit of difficulty with set_prop! though. The example is a bit over 80 lines of code, 30 lines of which are data. The example is based on MetaGraphsNext and implements a family tree.

The error is

ERROR: LoadError: MethodError: no method matching set_prop!(::MetaGraphsNext.MetaGraph{Int64, Graphs.SimpleGraphs.SimpleDiGraph{Int64}, Person, Nothing, Nothing, Nothing, MetaGraphsNext.var"#11#13", Float64}, ::Int64, ::Int64, ::Symbol, ::Symbol)

I believe the problem is that set_prop!(!Matched::AbstractMetaGraph{T}, ::Integer, ::Integer, ::Symbol, ::Any) where T isn't applicable because MetaGraphsNext.MetaGraph{Int64, Graphs.SimpleGraphs.SimpleDiGraph{Int64}, Person, Nothing, Nothing, Nothing, MetaGraphsNext.var"#11#13", Float64} is not a subtype of AbstractMetaGraph. The graph was constructed as MetaGraphsNext.MetaGraph(DiGraph(); label_type=Person).

Apparently I need to use all three of Graphs, MetaGraphs, and MetaGraphsNext because some functions are not exported from MetaGraphsNext.

I could post the code in this thread or open a PR to add it as an example in the docs directory.

@gdalle
Copy link
Contributor

gdalle commented Dec 15, 2023

add_vertex! adds a vertex to a graph. It does not return that vertex. How does the caller know what that vertex is so that it could add an edge or set a property?

It is definitely not clear enough in the docs, but most functions in Graphs.jl assume that the vertices are numbered from $1$ to $n$. That is why the default add_vertex!(g) takes no argument, because the added vertex is always $n+1$. Note that this gets trickier with rem_vertex!(g, v) because maintaining the invariant requires implicit numbering changes.

If you're interested, I have started a project GraphsInterfaceChecker.jl to formalize these implicit assumptions as proper Julia code that can be used to test new graph formats.

@gdalle
Copy link
Contributor

gdalle commented Dec 15, 2023

it seems I'm beiing encouraged to write documentation for a package I know very little about :-)

I agree it does look like a trap, but it turns out new users are often the best people to spot holes in the documentation, precisely because they're not accustomed to the code yet.

Ok, with your help I'll bite.

That's the spirit, thank you! :)

Apparently I need to use all three of Graphs, MetaGraphs, and MetaGraphsNext because some functions are not exported from MetaGraphsNext.

Actually the issue is that MetaGraphsNext is not a complement to MetaGraphs but a full replacement. That explains why MetaGraphs.set_prop! does not exist for the format MetaGraphsNext.MetaGraph.

In all likelihood, you should only use one of the two packages. If you don't care about performance, MetaGraphs.jl might be easier, but since it's the older and less efficient one, the documentation has received less love.

@MarkNahabedian
Copy link

MarkNahabedian commented Dec 15, 2023 via email

@MarkNahabedian
Copy link

I forked MetaGraphsNext to add documentation: https://github.com/MarkNahabedian/MetaGraphsNext.jl.

This test was failing on my fresh clone

    @testset verbose = false "Code formatting (JuliaFormatter.jl)" begin
        @test format(MetaGraphsNext; verbose=false, overwrite=false)
    end

so I commented it out for now.

I added a first draft of a new tutorial (Family Tree) that addresses the kinds of use cases I envision.

I wonder if there's a way to fix the URI in the README file to refer to the fork's github pages instead of the root's.

@gdalle
Copy link
Contributor

gdalle commented Dec 18, 2023

When a node is removed then other nodes are renumbered?

Indeed, in a Graphs.Simple(Di)Graph when you remove a node i it is permuted with the last one to retain a set of vertices corresponding to 1:n. But that is an "implementation detail", in the sense that other graph structures may choose to proceed differently

I can't imagine how I would function as a programmer in an environment where data objects don't have identity.

It sure is a strong argument for MetaGraphsNext, or for constructing a graph once and for all before analysis (which in some situations may not be feasible)

@gdalle
Copy link
Contributor

gdalle commented Dec 18, 2023

I forked MetaGraphsNext to add documentation: https://github.com/MarkNahabedian/MetaGraphsNext.jl.

Cool, thanks!
Wanna open a pull request so that we can discuss the changes there?

@MarkNahabedian
Copy link

JuliaGraphs/MetaGraphsNext.jl#75

I look forward to your comments there.

Copy link
Contributor

This pull request has been inactive for 30 days and will be automatically closed 7 days from now. If this pull request should not be closed, please either (1) fix the AutoMerge issues and re-trigger Registrator, which will automatically update the pull request, or (2) post a comment explaining why you would like this pull request to be manually merged. [noblock]

@github-actions github-actions bot added the stale label Jan 18, 2024
@MarkNahabedian
Copy link

With much help I was able to get MetaGraphsNext to work in my applications.

I'd like to withdraw this pull request, but I don't see a UI component for that.

@gdalle
Copy link
Contributor

gdalle commented Jan 18, 2024

You can just leave it there, it will close automatically.
I still haven't gotten around to reviewing your docs PR for MetaGraphsNext (thanks for that!) but it's on my to do list ☺️

@github-actions github-actions bot removed the stale label Jan 19, 2024
@giordano giordano deleted the registrator-nahagraphs-fd225a7d-v0.1.0-a6b2af7983 branch February 25, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants