feat: added simple check to graph_from_literal()

[schochastics]

Fix #824

The PR implements the suggested additional check if a graph is simple or not.
This required a lot of adjustments in the tests given that this means that the edge order was changed. I could imagine that this might lead to issues downstream so keeping this for after the 2.2 release.

cc @szhorvat is this what you intended?

[krlmlr]

Too risky, let's wait.

[schochastics]

@krlmlr Ok to merge this one too now?

[krlmlr]

Running revdepchecks now.

[schochastics]

I think there is one new failure because of this PR the other two new ones are from other breaking PRs we merged

[krlmlr]

Looks good. Can we also promote this check to simplify() directly? How does this work in igraph/C 1.0? I vaguely remember discussing this.

[schochastics]

Looks good. Can we also promote this check to simplify() directly? How does this work in igraph/C 1.0? I vaguely remember discussing this.

yes, and it looks worthwhile.

Where it'd go

Into the hand-written wrapper in R/simple.R

simplify <- function(graph, remove.multiple = TRUE, remove.loops = TRUE,
                     edge.attr.comb = igraph_opt("edge.attr.comb")) {
  if (is_simple(graph)) {
    return(graph)
  }
  simplify_impl(...)
}

Correctness

is_simple() is TRUE iff there are no loops and no multiple edges. In that state simplify() removes nothing regardless of remove.multiple / remove.loops / edge.attr.comb — so the early return is safe for every argument combination, not just the defaults. Input validation is preserved, since is_simple() already calls ensure_igraph() under the hood.

Performance (measured, 20k vertices / 100k edges)

	simple graph	non-simple graph
is_simple()	~0.003 ms	~0.007 ms
simplify()	~0.48 ms	~8.5 ms
guarded	~0.003 ms	~8.55 ms

is_simple() is ~150× cheaper than simplify() on a simple graph, and on a non-simple graph the extra check is ~0.08% overhead — effectively free in both directions.

One behavioral nuance

Today simplify() on an already-simple graph returns a content-equal but non-identical() fresh object (edges, attributes, and order all verified preserved). With the early return it returns the same object. Since igraph graphs are copy-on-modify in R, this is safe, and no existing test relies on the copy — the simplify() tests in test-attributes.R all use non-simple inputs, so the early-return path never triggers there.

Knock-on effect

If we promote it, the per-call-site guards become redundant and could revert to plain simplify(): the new graph_from_literal_i guard plus the existing calls in make.R, centrality.R (×2), and glet.R.

Only real trade-off is philosophical: simplify() would no longer guarantee a fresh object on already-simple input. Given copy-on-modify semantics that's not a practical concern.

@krlmlr should I move the check to that function? I actually would favor that

[krlmlr]

Go for it! (And why is touchstone unhappy?)

[krlmlr]

GHA workflow configs need to declare the permissions they need. Separate PR probably.

[schochastics]

Done. touchstone should be happy now too. Fixed it in separate PRs. merge if green?

[github-actions]

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 6448626 is merged into main:

• ✔️as_adjacency_matrix: 743ms -> 747ms [-0.28%, +1.5%]
• ✔️as_biadjacency_matrix: 755ms -> 756ms [-0.81%, +1.15%]
• ✔️as_data_frame_both: 1.7ms -> 1.7ms [-7.29%, +7.83%]
• ✔️as_long_data_frame: 4.17ms -> 4.16ms [-2.01%, +1.38%]
• ✔️es_attr_filter: 2.99ms -> 2.95ms [-4.17%, +2.01%]
• ✔️graph_from_adjacency_matrix: 119ms -> 118ms [-2.89%, +0.95%]
• ✔️graph_from_data_frame: 3.67ms -> 3.63ms [-2.41%, +0.76%]
• ✔️vs_attr_filter: 1.76ms -> 1.72ms [-5.06%, +0.41%]
• ✔️vs_by_name: 1.12ms -> 1.13ms [-2.76%, +4.75%]
  Further explanation regarding interpretation and methodology can be found in the documentation.

[krlmlr]

Will run revdepchecks combined.