Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: New movement implementation #123

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

kklimonda-cl
Copy link
Contributor

@kklimonda-cl kklimonda-cl commented Jul 31, 2024

Description

This new algorithm implements generation of move actions by calculating steps between existing and expected sequences:

  • First, based on the position type, we generate an expected sequence, describing the expected order of the elements.
  • Next, GenerateMovements() create a set of all movements required to transform existing sequences into expected sequence.
  • finally, OptimizeMovements() simulates movements to remove actions that will have no effect.

@kklimonda-cl kklimonda-cl marked this pull request as draft July 31, 2024 11:37
@kklimonda-cl
Copy link
Contributor Author

kklimonda-cl commented Jul 31, 2024

longestCommonSubsequence is not the right name, as this is really a variation on LongestCommonSubstring algorithm, but instead of strings we want to handle slices of Movable types.

It's also pretty inefficient when generating the longest common sequence as we constantly reallocate results, and we should be reusing that array (after preallocation).

@kklimonda-cl
Copy link
Contributor Author

kklimonda-cl commented Aug 1, 2024

Added an optimization pass:

To simplify the move actions generation, algorithm doesn't check if any previous move made current one reduntant. So for some sequences it can generate extra movements.
For example, with the existing list '(A B C D E) and given entries list '(C B) with position PositionBefore{Directly: false, Pivot: D} the following expected list is generated '(A C B D E) and we generate two movements '((C before D)(B after C)) but the optimization can be applied to transform it into just '((B after C)).
This is done in a separate step, where we take existing list and simulate all moves one by one, verifying which moves can be skipped.

@kklimonda-cl kklimonda-cl linked an issue Aug 1, 2024 that may be closed by this pull request
@kklimonda-cl kklimonda-cl changed the title feat: LCS-based movement implementation feat: New movement implementation Aug 2, 2024
Base automatically changed from feat-render-resource-second-part to main August 8, 2024 12:53
@kklimonda-cl kklimonda-cl changed the base branch from main to feat-sdk-manager September 3, 2024 08:47
@kklimonda-cl kklimonda-cl marked this pull request as ready for review September 3, 2024 11:34
@kklimonda-cl kklimonda-cl changed the base branch from feat-sdk-manager to bug-panos-template-imports September 3, 2024 11:34
@kklimonda-cl kklimonda-cl force-pushed the bug-panos-template-imports branch 2 times, most recently from d61ac46 to 10b22cc Compare September 6, 2024 11:27
Base automatically changed from bug-panos-template-imports to main September 6, 2024 11:35
This new algorithm implements generation of move actions by utilizing a longest common substring algorithm (LCS).

First, based on the position type, we generate a list describing the expected order of the elements.

LCS algorithm is then used to find the longest sequence of items that is shared between the expected list, and an actual
list (e.g. a list of entries from the server).

Once longest sequence is known, we figure out the least amount of moves to translate existing list into its expected
form, and those movements are returned back.
Fixed some bugs in LCS implementation, but it needs to be be optimized - it takes ~25 seconds to find a common
subsequence in two equal sequences of 50k elements.

Added an OptimizeMovements step that is done after GenerateMovements returns a list of all move actions. It removes
redundant actions that, after previous actions were applied, no longer have any effect.
…tion

Now GenerateMovements() generate all movements blindly, and depend on the
optimization done by OptimizeMovements() to remove reduntant actions.
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.

MoveGroup logic enhancements
1 participant