feat: Add support for HIF format #53

AleksanderWWW · 2025-03-19T13:45:03Z

Work in progress.

TODO:

implement hq_load function with HIF_Format switch
implement hq_save likewise
add tests

AleksanderWWW · 2025-03-20T12:04:57Z

Seems that the first phase is ready.
It's far from perfect, so looking forward to a brutal review :)

pszufe

Thank you, see the comments.

pszufe · 2025-03-20T13:23:45Z

test/runtests.jl

@@ -73,6 +73,10 @@ h1[5,2] = 6.5
        @test get_vertex_meta(h1, 1) == get_vertex_meta(loaded_hg, 1)
        @test get_hyperedge_meta(h1, 2) == get_hyperedge_meta(loaded_hg, 2)



Make unit tests also on data from the https://github.com/pszufe/HIF-standard/tree/main/tests/test_files/HIF-compliant from the HIF-standard libraries.
Make a copy of those files to test/data/HIF-standard folder in the package repository.
Provide in the above folder README.md explaining the files come from the above library.

Each of the files should be able to load and later save.
While saving provide code comparing both JSON outputs (parse both JSONs as Dicts and compare the Dicts)

I understand we still do not have yet code that iterates over HIF test files.

How to handle the fact, that in order to load such a file I need to explicitly pass the types of values therein?

I. e. is manually loading each file separately the only way forward? I don't see how I can dynamically do it in a loop iterating over the directory contents

pszufe · 2025-03-20T13:27:17Z

src/io.jl

+    _ = format
+
+    json_hg = Dict{Symbol, Any}()
+    incidences = []


pls no untyped vectors /containers
https://docs.julialang.org/en/v1/manual/performance-tips/#man-performance-abstract-container
most likely this are something like Union{String, Int, Nothing} (I am not sure - please check what types are possible)

pszufe · 2025-03-20T13:29:00Z

src/io.jl

+    he_meta = h.he_meta
+
+    if any(isnothing, h.v_meta)
+        v_meta = [i for i in 1:length(h.v_meta)]


what is the intention of this code?
SimpleHypegraphs supports both Nothing as type of metadata (in that case there is no metadata) as well as concrete types

Yes, but as far as I understand, we cannot have Nothing in edge or node metadata in the HIF format. This is probably not the best take, but the idea was that if there are missing metadata points, then we need to fallback to the solution, where edge or node ids are indexes in the matrix

pszufe · 2025-03-20T13:33:56Z

src/io.jl

+    edge_dict = Dict(i => val for (i, val) in pairs(he_meta))
+
+
+    types = collect(typeof(h).parameters)


most likely yo wanted to have those in the method definition:

hg_save(io::IO, h::Hypergraph{T, V, E, D}, format::HIF_Format) where {T, V, E, D}

Check how parametric types work in Julia:
https://docs.julialang.org/en/v1/manual/types/

pszufe · 2025-03-20T13:37:13Z

src/io.jl

+    incidences = []
+    v_meta = h.v_meta
+    he_meta = h.he_meta
+


do not change type of a variable
https://docs.julialang.org/en/v1/manual/performance-tips/#Avoid-changing-the-type-of-a-variable

pszufe · 2025-03-20T13:39:39Z

src/io.jl

+    E = types[3]
+
+    for node_idx in 1:length(v_meta)
+        for edge_idx in 1:length(he_meta)


use eachindex as this is the recommended syntax
https://discourse.julialang.org/t/best-practices/85138

pszufe · 2025-03-20T13:43:29Z

src/io.jl

+    for node_idx in 1:length(v_meta)
+        for edge_idx in 1:length(he_meta)
+            node = node_dict[node_idx]
+            if V == String


dispatch on type (use multiple-dispatch property of the language)
BTW there is string(xs...) method - so string() works with any type.

pszufe · 2025-03-20T13:46:28Z

src/io.jl

+
+            weight = h[node_idx, edge_idx]
+
+            if isnothing(weight)


iterating over an entire sparse matrix is extremely inefficient (see the nested loop above)
Use gethyperedges or getvertices methods.

pszufe · 2025-03-20T13:53:21Z

src/io.jl

+    format::HIF_Format;
+    T::Type{U} = Bool,
+    D::Type{<:AbstractDict{Int, U}} = Dict{Int, T},
+    V::Union{Type{String}, Type{Int}} = String,


since V and E are parametric the code should perform type casting when collecting their values from JSON.
So this should be V(val) or E(val). For string you will need to handle this separately as for the String target the string method should be called.

pszufe · 2025-03-20T13:53:51Z

src/io.jl

+
+    data = JSON3.read(read(io, String))
+
+    nodes = get(data, "nodes", [])


no untyped containers pls (see the comment above)

pszufe · 2025-03-28T15:58:38Z

src/SimpleHypergraphs.jl

@@ -74,6 +74,10 @@ end
 include("hypergraph.jl")
 include("io.jl")

+include("hif/hif.jl")


Let's just make those three files into one named io_hif.jl without creating the hif subfolder.
In Julia packages we do not try to keep one mehod per file

pszufe · 2025-03-28T16:12:34Z

src/hif/hif_load.jl

+    T::Type{U},
+    D::Type{<:AbstractDict{Int,U}},
+) where {U<:Real}
+    node_metadata = Vector{Union{JSON3.Object, Nothing}}()


You do not want the user suddenly see some JSON3.Object just cast it to a String.
Additionally, the code could check if this is some more complex data structure and than make it into a Dict.
Perhaps what could help would be using second parameter for reading JSON3.read("test.json", Dict{String, Any})

pszufe · 2025-03-28T16:14:10Z

src/hif/hif_load.jl

+        append!(edge_metadata, [nothing for _ in 1:k])
+    end
+
+    return Hypergraph{T,JSON3.Object,JSON3.Object,D}(n, k, node_metadata, edge_metadata)


User wants to get as metadata parametric types something like String or Dict{String, String} for more complex metadata. I think we do not need to support deeper nesting than one level.

pszufe · 2025-03-28T16:15:38Z

src/hif/hif_load.jl

+
+function add_weights_from_incidences!(
+    h::Hypergraph, 
+    incidences::JSON3.Array{JSON3.Object},


those types will need to be updated once you sort out the way you read the data.
Also using just AbstractVector as function type argument in such places is good enough.

pszufe · 2025-03-28T16:17:35Z

test/runtests.jl

@@ -73,6 +73,10 @@ h1[5,2] = 6.5
        @test get_vertex_meta(h1, 1) == get_vertex_meta(loaded_hg, 1)
        @test get_hyperedge_meta(h1, 2) == get_hyperedge_meta(loaded_hg, 2)



I understand we still do not have yet code that iterates over HIF test files.

AleksanderWWW added 5 commits March 19, 2025 14:42

feat: Add support for HIF format

c620364

add function to parse input for hg dimensions

75208fd

finalize PoC for hg loading

7694c4a

add function for saving hg in the hiv format

ef22632

add test

94b02b0

AleksanderWWW marked this pull request as ready for review March 20, 2025 12:03

AleksanderWWW added 2 commits March 20, 2025 14:44

move to a separate file

b491ee1

even out the diff

cf48d52

pszufe reviewed Mar 20, 2025

View reviewed changes

AleksanderWWW added 4 commits March 20, 2025 16:10

code review 1

fa7f526

code review 2

e71787c

restructure + better metadata handling in load

f069829

improve saving

8fa099d

pszufe requested changes Mar 28, 2025

View reviewed changes

AleksanderWWW added 12 commits March 31, 2025 21:14

improve loading

94ce937

update

c4c8be2

Merge branch 'master' into aw/hif-import-export

c29e9ad

update

ae33a1e

update

f716a3f

update

d7908dd

start adding tests

5b470ff

start refactor

c13fe0e

update

d3bb227

fix for no incidences and edges as string

d1547d0

add saving and tests

60dadb0

fix last corner case

64f51b3

colltoaction mentioned this pull request Jun 4, 2025

Hypergraph Interchange Format invitation HypergraphRepository/datasets#11

Open

update

64eb4bd

		@@ -73,6 +73,10 @@ h1[5,2] = 6.5
		@test get_vertex_meta(h1, 1) == get_vertex_meta(loaded_hg, 1)
		@test get_hyperedge_meta(h1, 2) == get_hyperedge_meta(loaded_hg, 2)

		edge_dict = Dict(i => val for (i, val) in pairs(he_meta))


		types = collect(typeof(h).parameters)


		data = JSON3.read(read(io, String))

		nodes = get(data, "nodes", [])

feat: Add support for HIF format #53

Are you sure you want to change the base?

feat: Add support for HIF format #53

Uh oh!

Conversation

AleksanderWWW commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AleksanderWWW commented Mar 20, 2025

Uh oh!

pszufe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AleksanderWWW commented Mar 19, 2025 •

edited

Loading