Skip to content

Conversation

@brunos252
Copy link
Contributor

@brunos252 brunos252 commented May 2, 2022

Description

Recursive DFS used for finding flows in the Max Flow algorithm was failing when returning from a recursive call for multiple levels. path = path[:-2] simply reassigned the variable inside that function call, and didn't modify the list itself, leaving edges in path that shouldn't be there. del path[-2:] modifies the list at the original address, which is the wanted functionality.

Pull request type

  • Bugfix
  • Algorithm/Module
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Related issues

#140

######################################

Reviewer checklist (the reviewer checks this part)

Module/Algorithm

  • Core algorithm/module implementation
  • Query module implementation
  • Unit tests
  • End-to-end tests
  • Code documentation
  • README short description
  • Documentation on memgraph/docs

######################################

@brunos252 brunos252 added the status: ready PR is ready for review label May 2, 2022
@brunos252 brunos252 self-assigned this May 2, 2022
@brunos252 brunos252 linked an issue May 2, 2022 that may be closed by this pull request
@antoniofilipovic antoniofilipovic added status: ship it PR approved and removed status: ready PR is ready for review labels May 4, 2022
Copy link
Contributor

@antoniofilipovic antoniofilipovic left a comment

Choose a reason for hiding this comment

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

Note, add E2E test for this case before merging it, as noted on Slack by @jmatak

@antoniofilipovic antoniofilipovic added status: change PR reviewed - needs changes and removed status: ship it PR approved labels May 4, 2022
@brunos252 brunos252 added status: ready PR is ready for review and removed status: change PR reviewed - needs changes labels May 5, 2022
@antoniofilipovic antoniofilipovic added status: ship it PR approved and removed status: ready PR is ready for review labels May 6, 2022
@brunos252 brunos252 requested review from antoniofilipovic and jmatak and removed request for antoniofilipovic May 6, 2022 11:58
@jmatak jmatak merged commit 470816e into main May 6, 2022
@jmatak jmatak deleted the max-flow-fix branch May 6, 2022 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ship it PR approved

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[BUG] Sample query for "Europe gas pipelines" dataset doesn't work

5 participants