Skip to content

Commit

Permalink
fix corner case in normalize_graphdata (#589)
Browse files Browse the repository at this point in the history
  • Loading branch information
CarloLucibello authored Feb 6, 2025
1 parent cb2ad5f commit 58fcd7d
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 14 deletions.
8 changes: 3 additions & 5 deletions GNNGraphs/src/gnngraph.jl
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,9 @@ function GNNGraph(data::D;
edata = normalize_graphdata(edata, default_name = :e, n = num_edges,
duplicate_if_needed = true)

# don't force the shape of the data when there is only one graph
gdata = normalize_graphdata(gdata, default_name = :u,
n = num_graphs > 1 ? num_graphs : -1)
gdata = normalize_graphdata(gdata, default_name = :u, n = num_graphs, glob=true)

GNNGraph(graph,
return GNNGraph(graph,
num_nodes, num_edges, num_graphs,
graph_indicator,
ndata, edata, gdata)
Expand Down Expand Up @@ -203,7 +201,7 @@ function GNNGraph(g::GNNGraph; ndata = g.ndata, edata = g.edata, gdata = g.gdata
ndata = normalize_graphdata(ndata, default_name = :x, n = g.num_nodes)
edata = normalize_graphdata(edata, default_name = :e, n = g.num_edges,
duplicate_if_needed = true)
gdata = normalize_graphdata(gdata, default_name = :u, n = g.num_graphs)
gdata = normalize_graphdata(gdata, default_name = :u, n = g.num_graphs, glob=true)

if !isnothing(graph_type)
if graph_type == :coo
Expand Down
3 changes: 1 addition & 2 deletions GNNGraphs/src/gnnheterograph/gnnheterograph.jl
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ function GNNHeteroGraph(data::EDict;
ndata = normalize_heterographdata(ndata, default_name = :x, ns = num_nodes)
edata = normalize_heterographdata(edata, default_name = :e, ns = num_edges,
duplicate_if_needed = true)
gdata = normalize_graphdata(gdata, default_name = :u,
n = num_graphs > 1 ? num_graphs : -1)
gdata = normalize_graphdata(gdata, default_name = :u, n = num_graphs, glob = true)
end

return GNNHeteroGraph(graph,
Expand Down
19 changes: 12 additions & 7 deletions GNNGraphs/src/utils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -129,19 +129,24 @@ function normalize_graphdata(data; default_name::Symbol, kws...)
normalize_graphdata(NamedTuple{(default_name,)}((data,)); default_name, kws...)
end

function normalize_graphdata(data::NamedTuple; default_name, n, duplicate_if_needed = false)
function normalize_graphdata(data::NamedTuple; default_name::Symbol, n::Int,
duplicate_if_needed::Bool = false, glob::Bool = false)
# This had to workaround two Zygote bugs with NamedTuples
# https://github.com/FluxML/Zygote.jl/issues/1071
# https://github.com/FluxML/Zygote.jl/issues/1072
# https://github.com/FluxML/Zygote.jl/issues/1072 # TODO fixed. Can we simplify something?


if n > 1
@assert all(x -> x isa AbstractArray, data) "Non-array features provided."
end

if n <= 1
# If last array dimension is not 1, add a new dimension.
# This is mostly useful to reshape global feature vectors
# of size D to Dx1 matrices.
if n <= 1 && glob == true
@assert n == 1
n = -1 # relax the case of a single graph, allowing to store arbitrary types
# # # If last array dimension is not 1, add a new dimension.
# # # This is mostly useful to reshape global feature vectors
# # # of size D to Dx1 matrices.
# TODO remove this and handle better the batching of global features
unsqz_last(v::AbstractArray) = size(v)[end] != 1 ? reshape(v, size(v)..., 1) : v
unsqz_last(v) = v

Expand All @@ -161,7 +166,7 @@ function normalize_graphdata(data::NamedTuple; default_name, n, duplicate_if_nee

for x in data
if x isa AbstractArray
@assert size(x)[end]==n "Wrong size in last dimension for feature array, expected $n but got $(size(x)[end])."
@assert size(x)[end] == n "Wrong size in last dimension for feature array, expected $n but got $(size(x)[end])."
end
end
end
Expand Down
11 changes: 11 additions & 0 deletions GNNGraphs/test/gnngraph.jl
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,17 @@ end
@test g.num_nodes == 1
@test g.num_edges == 0
@test g.ndata.a == [1]

g = GNNGraph((Int[], Int[]); ndata=(;a=[1]), edata=(;b=Int[]), num_nodes=1)
@test g.num_nodes == 1
@test g.num_edges == 0
@test g.ndata.a == [1]
@test g.edata.b == Int[]

g = GNNGraph(; edata=(;b=Int[]))
@test g.num_nodes == 0
@test g.num_edges == 0
@test g.edata.b == Int[]
end


Expand Down

2 comments on commit 58fcd7d

@CarloLucibello
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JuliaRegistrator register subdir=GNNGraphs

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registration pull request created: JuliaRegistries/General/124499

Tip: Release Notes

Did you know you can add release notes too? Just add markdown formatted text underneath the comment after the text
"Release notes:" and it will be added to the registry PR, and if TagBot is installed it will also be added to the
release that TagBot creates. i.e.

@JuliaRegistrator register

Release notes:

## Breaking changes

- blah

To add them here just re-invoke and the PR will be updated.

Tagging

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a GNNGraphs-v1.4.2 -m "<description of version>" 58fcd7df11333adc009cd0f07bfdc5751775042a
git push origin GNNGraphs-v1.4.2

Please sign in to comment.