Skip to content

Set prev for nodes marked -inf in BellmanFord - #325

Closed
ItsDeadlyProgrammer wants to merge 1 commit into
kth-competitive-programming:mainfrom
ItsDeadlyProgrammer:bellmanford-prev
Closed

ItsDeadlyProgrammer wants to merge 1 commit into
kth-competitive-programming:mainfrom
ItsDeadlyProgrammer:bellmanford-prev

Conversation

@ItsDeadlyProgrammer

Copy link
Copy Markdown

Fixes #300.

Nodes that are only reached through a negative cycle get dist = -inf in the second loop, but their prev is never touched there, so they end up with prev = -1 (or a prev pointing at a node with finite distance, left over from the main loop). This sets prev = e.a in that loop as suggested in the issue. Line count of the header is unchanged.

There was no stress test for BellmanFord, so I added stress-tests/graph/BellmanFord.cpp. It runs random graphs with negative edges/cycles and checks:

  • dist matches a naive V-1 rounds + propagation implementation
  • every reachable node other than the source has prev != -1, and prev -> v is a real edge
  • finite nodes: dist[prev] + w == dist[v]; -inf nodes: dist[prev] == -inf

On current main, 100k cases give 1583 -inf nodes with prev = -1 and 340 with a prev that is not -inf; distances were always correct. With the change, all checks pass (also tried up to 60 nodes / 150 edges).

Nodes that only get dist = -inf in the propagation loop kept prev = -1
(or a stale prev pointing at a finite node). Set prev there too, so every
reachable node's prev is a real in-neighbour: on a shortest path for
finite nodes, and a -inf node for -inf nodes.

Also add a stress test for BellmanFord, checking dist against a naive
implementation and checking prev.

Fixes kth-competitive-programming#300
@simonlindholm

Copy link
Copy Markdown
Member

I don't think the -1 case can happen, because what would it mean for a node to only be reachable from a negative cycle? And then what does par actually mean for a node in a negative weight cycle? If you take the interpretation that it should mean a possible parent for an arbitrarily-short finite path, then yes, I guess this improves matters. But it doesn't e.g. give you the ability to follow parent pointers to find negative weight cycles, which if anything is probably what you want to use parents for. (Their usefulness is a bit unclear to me in general.)

@ItsDeadlyProgrammer

Copy link
Copy Markdown
Author

Yeah, you're right. I went back and checked, and the -1 case only shows up for the source itself when it's on a negative cycle, which is expected. The numbers in my description were basically all that case, my bad.

Fair point on the cycles too. I tried following prev from -inf nodes with this change and about half the time you end up in a cycle that isn't negative, so it doesn't really help there. I'll close this, thanks for taking a look!

@JJCUBER

JJCUBER commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

I don't think the -1 case can happen, because what would it mean for a node to only be reachable from a negative cycle? And then what does par actually mean for a node in a negative weight cycle? If you take the interpretation that it should mean a possible parent for an arbitrarily-short finite path, then yes, I guess this improves matters. But it doesn't e.g. give you the ability to follow parent pointers to find negative weight cycles, which if anything is probably what you want to use parents for. (Their usefulness is a bit unclear to me in general.)

I wrote the original issue/fix a long time ago, but if I remember correctly, my "fix" (in #300) allows for finding any node with a $-\infty$ weight and following parents to eventually find a negative cycle. I want to say this was related to CSES's Cycle Finding problem. I believe that the final state currently is an amalgamation due to how infinities have propagated partially outside of the actual negative cycle but not fully. It's possible that this isn't fully correct because the parents behave weird in general (and I agree, their use is dubious at best).

@ItsDeadlyProgrammer

Copy link
Copy Markdown
Author

Thanks for the context! I tested this on ~200k random graphs and it seems to go the other way. On current main, following prev n times from any node that gets -inf in the main relaxation loop always lands on a negative cycle. With the #300 change, the -inf propagation loop overwrites prev for nodes on the cycle too, and only about half of -inf nodes lead to a negative cycle. So the current code already supports Cycle Finding, and #300 could probably be closed.

@JJCUBER

JJCUBER commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context! I tested this on ~200k random graphs and it seems to go the other way. On current main, following prev n times from any node that gets -inf in the main relaxation loop always lands on a negative cycle. With the #300 change, the -inf propagation loop overwrites prev for nodes on the cycle too, and only about half of -inf nodes lead to a negative cycle. So the current code already supports Cycle Finding, and #300 could probably be closed.

Interesting, I really wish I remembered what issue I was running into with the original method. I guess I'll close it and reopen it if I ever remember what edge case I ran into (I know there was some issue, I just don't recall it unfortunately).

@simonlindholm

Copy link
Copy Markdown
Member

(Sorry for being bad at getting around to dealing with KACTL issues sometimes! I don't do much competitive programming nowadays, so it takes more effort to get back into it.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bellman Ford's prev doesn't get properly updated

3 participants