Implement Smooth Sort #5236

JAPNITSINGH · 2024-06-18T09:37:25Z

I have read CONTRIBUTING.md.
This pull request is all my own work -- I have not plagiarized it.
All filenames are in PascalCase.
All functions and variable names follow Java naming conventions.
All new algorithms have a URL in their comments that points to Wikipedia or other similar explanations.
All new code is formatted with clang-format -i --style=file path/to/your/file.java

next step is to improve it to leonardo sort

…eckstyle faults

JAPNITSINGH · 2024-06-18T10:21:23Z

Additionaly done the following to fix pervious issues:

I have run mvn pmd:check to ensure there are no pmd errors
I have run mvn checkstyle:checkstyle to ensure correct style is followed

vil02 · 2024-06-18T15:13:32Z

src/main/java/com/thealgorithms/sorts/SmoothSort.java

Is it possible to derive your class from SortAlgorithm?

vil02 · 2024-06-18T15:16:48Z

src/test/java/com/thealgorithms/sorts/SmoothSortTest.java

If the answer to the above questions is yes, then if should be enough to derive the SmoothSortTest from SortingAlgorithmTest.

I believe it is possible, I just have to use generic T[] in place of Integer[] modify the code a little bit and use .compareTo() and it should be good. I shall work on it and update this PR. I'll comment here once changes are ready.

I have updated the code, haven't pushed it yet. The smooth sort algorithm I implemented seems to be failing for most arrays of size greater than 700. I guess the problem is with how I am calculating the left child of a node. I am having a look into it. I shall provide an update here once I am done with the fix

I have updated to code, derived the class from SortAlgorithm, removed unnecessary return statements , renamed variables to make the code more understandable.

… Branch_Smooth_Sort

codecov-commenter · 2024-06-24T12:37:06Z

Codecov Report

Attention: Patch coverage is 98.21429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 40.24%. Comparing base (f279f9d) to head (d5e3b95).

Files	Patch %	Lines
.../main/java/com/thealgorithms/sorts/SmoothSort.java	98.21%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #5236      +/-   ##
============================================
+ Coverage     39.85%   40.24%   +0.38%     
- Complexity     2437     2470      +33     
============================================
  Files           517      518       +1     
  Lines         15471    15583     +112     
  Branches       2958     2981      +23     
============================================
+ Hits           6166     6271     +105     
- Misses         9016     9018       +2     
- Partials        289      294       +5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

vil02

At the moment the code is way too complex. I would suggest to:

implement LeonardoHeap (or use one of the existing implementations in heaps),
use it in the implementation of SmoothSort.

vil02 · 2024-06-24T16:58:40Z

src/main/java/com/thealgorithms/sorts/SmoothSort.java

+
+                    int indexOfRightChild = rootNodeIndices.get(j) - 1; // right child is of level n-2
+                    int indexOfLeftChild = rootNodeIndices.get(j) - 1 - getLeonardoNumbers()[currentLeonardoLevel - 2];
+                    if (array[prevRootNodeIndex].compareTo(array[indexOfRightChild]) > 0 && array[prevRootNodeIndex].compareTo(array[indexOfLeftChild]) > 0) {


As far as I see there is no test with array[prevRootNodeIndex].compareTo(array[indexOfRightChild]) > 0 evaluating to true and array[prevRootNodeIndex].compareTo(array[indexOfLeftChild]) > 0 to false.

To ensure that the logic is correct, please add a missing test.

Sure, I shall add a specific test case in SmoothSortTest.java.

I see why this is happening and I shall try my best to explain in this comment.

Leonardo Heap has the following three properties that define it (Going through the array from left to right):

The trees are in decreasing order of levels

Root nodes of the trees are in increasing order

Each tree is in a 'max-heap' state.

The Level-1 leonardo Tree(L1) and Level 0 leonardo Tree (L0) are the 'base case' trees as they have exactly one node.

At any given valid leonardo heap, if the heap contains both L1 and L0 tree then the element in L0 tree will always be greater than element at L1 tree, because of the second property mentioned above ( L0 >= L1 )

When L2 tree is being constructed, the level0 tree becomes the right child level1 tree becomes the left child. And hence it is not possible to have a condition such that level0 < prevRootNodeVal < level1 as it would be a voilation of the above.

This section of the wiki page and this article (under the imageAn erroneous swap ) mention that root needs to be bigger than the new element and the roots of its child nodes for a swap to happen. However this article(in page 6) mentions that the preRootValue should exceede its largest son.

I believe the statement in wiki is more genreic statement than a technical one and this condition can be optimized to just having array[prevRootNodeIndex].compareTo(array[indexOfLeftChild]) > 0. I shall however look around for more articles that suggest otherwise before I make changes to this.

vil02 · 2024-06-24T17:00:07Z

src/main/java/com/thealgorithms/sorts/SmoothSort.java

+        Integer[] leonardoNumbers = {1, 1, 3, 5, 9, 15, 25, 41, 67, 109, 177, 287, 465, 753, 1219, 1973, 3193, 5167, 8361, 13529, 21891, 35421, 57313, 92735, 150049, 242785, 392835, 635621, 1028457, 1664079, 2692537, 4356617, 7049155, 1405773, 18454929, 29860703, 48315633, 78176337, 126491971,
+            204668309, 331160281, 535828591};


Why not to use LeonardoNumber here?

The LeonardoNumber class is recursively calculating the Leonardo Numbers. I did not want to calculate(or re-calculate) the numbers everytime. I shall modify my code and use LeonardoNumber to store the results in an array before I start creating the heap and use the results stored in that array when needed.

@vil02 I however think recrsivly calculating these numbers ( before or during the execution ) might not be a great idea and would just increase the execution time as it will run the same recursion stacks multiple times. If you still suggest using LeonardoNumber.java I shall go ahead and make changes as mentioned above.

vil02 · 2024-06-24T17:00:57Z

src/main/java/com/thealgorithms/sorts/SmoothSort.java

+                    int indexOfRightChild = rootNodeIndices.get(j) - 1; // right child is of level n-2
+                    int indexOfLeftChild = rootNodeIndices.get(j) - 1 - getLeonardoNumbers()[currentLeonardoLevel - 2];
+                    if (array[prevRootNodeIndex].compareTo(array[indexOfRightChild]) > 0 && array[prevRootNodeIndex].compareTo(array[indexOfLeftChild]) > 0) {
+                        swap(array, prevRootNodeIndex, currentRootNodeIndex);


Suggested change

swap(array, prevRootNodeIndex, currentRootNodeIndex);

SortUtils.swap(array, prevRootNodeIndex, currentRootNodeIndex);

vil02 · 2024-06-24T17:01:16Z

src/main/java/com/thealgorithms/sorts/SmoothSort.java

+                    }
+                } else {
+                    // swap
+                    swap(array, prevRootNodeIndex, currentRootNodeIndex);


Suggested change

swap(array, prevRootNodeIndex, currentRootNodeIndex);

SortUtils.swap(array, prevRootNodeIndex, currentRootNodeIndex);

vil02 · 2024-06-24T17:01:34Z

src/main/java/com/thealgorithms/sorts/SmoothSort.java

+
+    private static <T> void swap(T[] array, int idx, int idy) {
+        T swap = array[idx];
+        array[idx] = array[idy];
+        array[idy] = swap;
+    }


Suggested change

private static <T> void swap(T[] array, int idx, int idy) {

T swap = array[idx];

array[idx] = array[idy];

array[idy] = swap;

}

vil02 · 2024-06-24T17:01:55Z

src/main/java/com/thealgorithms/sorts/SmoothSort.java

+        }
+
+        if (array[childIndexForSwap].compareTo(array[currentRootNodeIndex]) > 0) {
+            // swap(And keep on swapping I guess, I did not implement that which might be causing issue?)


This comment was added during debugging, I shall remove this in the next commit.

vil02 · 2024-06-24T17:03:10Z

src/main/java/com/thealgorithms/sorts/SmoothSort.java

+                    // swap
+                    swap(array, prevRootNodeIndex, currentRootNodeIndex);


Suggested change

// swap

swap(array, prevRootNodeIndex, currentRootNodeIndex);

SortUtils.swap(array, prevRootNodeIndex, currentRootNodeIndex);

No need for the comment. The name of the method swap is pretty clear.

vil02 · 2024-06-24T17:03:43Z

src/main/java/com/thealgorithms/sorts/SmoothSort.java

+                    leonardoTreeLevelforHeapify = currentLeonardoTreeLevels[j - 1];
+                }
+                j = j - 1;
+                if (j == i - 1) {


j == i - 1 is never false (among the existing test cases).

I do have a test case (not currently committed) with around 900 entries where this evalueates to false during creation of the heap. I can try to reduce the entires in such a way that the condition still evaluates to true, I will for sure end up having an array of size 20-30 ( as i will need 3 trees of non consecutive levels .... L1 L3 and L5 at least ).

Would it be fine if I added a test case in SmoothSortTest.java with around 20-30 elements in the test? If yes, I shall work on it at the very end after resolving the other conversations.

Update: Yes, this condition is never false. I belive what I intented to do was run maxHeapifyLeonardoTree if you reached the left most tree and further swaps are still present.

I am checking this as well.

vil02 · 2024-06-24T17:03:58Z

src/main/java/com/thealgorithms/sorts/SmoothSort.java

+        // maxHeapifyLeonardoTree(rightChildIndex, currentLeonardoLevel - 2, array);
+        // maxHeapifyLeonardoTree(leftChildIndex, currentLeonardoLevel - 1, array);


This too was added for debugging, I shall remove this in my next commit. please ignore this.

vil02 · 2024-06-24T17:05:48Z

src/main/java/com/thealgorithms/sorts/SmoothSort.java

+            if (consecutiveTreeIndices[0] != -1) {
+                // if 0th or 1st index is -1 that implies there are no concequtive trees
+                leonardoLevelTracker = leonardoLevelTracker & ~(1 << consecutiveTreeIndices[0]);
+                leonardoLevelTracker = leonardoLevelTracker & ~(1 << consecutiveTreeIndices[1]);
+                leonardoLevelTracker = leonardoLevelTracker | (1 << consecutiveTreeIndices[1] + 1);
+            } else if ((leonardoLevelTracker & 2) == 0) {
+                leonardoLevelTracker = leonardoLevelTracker | (1 << 1);
+            } else {
+                leonardoLevelTracker = leonardoLevelTracker | (1 << 0);
+            }


Is it possible to apply the SingleBitOperations (or something from bitmanipulation)?

I did not come accross this class earlier.
Yes, this class can definately be used. I shall modify this section of the code as well.

JAPNITSINGH · 2024-06-25T05:37:57Z

At the moment the code is way too complex. I would suggest to:

implement LeonardoHeap (or use one of the existing implementations in heaps),

use it in the implementation of SmoothSort.

I did try to use existing implementaions of Heaps, however Leonardo Heap is a bit different from the others since this heap contains multiple loenardo trees of very specific shapes and the root node being the right most elemnent in the list.

I can crate a new class LeonardoHeap in heaps, however I believe I might not endup using the interface Heap.java in this implementation, would that be alright?

… Branch_Smooth_Sort

JAPNITSINGH · 2024-07-03T08:23:09Z

Adding this comment to provide updates and ensure this PR is not marked as stale:

I have refactored the code, not yet pushed as I want to do some more changes and try to use the interface Heap.java
I shall comment here once the changes are ready to be reviewed.

JAPNITSINGH added 7 commits May 28, 2024 14:29

Adding Skeleton

9e5cb32

Understanding Heap Sort

9ff4e85

Implement merge sort

69b24a8

next step is to improve it to leonardo sort

Smooth Sort impl, needs cleaning.

6de4059

Added test cases, improved code coverage.

d6a14b6

Refactorization of code

384f814

Merge branch 'TheAlgorithms:master' into Branch_Smooth_Sort

29252e1

JAPNITSINGH requested review from yanglbme, vil02 and BamaCharanChhandogi as code owners June 18, 2024 09:37

JAPNITSINGH added 3 commits June 18, 2024 15:24

Fixed brackets and ran mvn checkstyle:checkstyle to ensure no more ch…

4f0fd70

…eckstyle faults

Fixed a tab space issue

0cdcba2

Removed useless parenthesis.

d32e435

vil02 requested changes Jun 18, 2024

View reviewed changes

JAPNITSINGH added 3 commits June 24, 2024 18:04

Re work, fix bugs, names etc.

712868e

Merge branch 'TheAlgorithms:master' into Branch_Smooth_Sort

368db8a

Merge branch 'Branch_Smooth_Sort' of github.com:JAPNITSINGH/Java into…

5f9d2e7

… Branch_Smooth_Sort

JAPNITSINGH added 3 commits June 24, 2024 18:21

Forgot to add test file

990b183

Improved code coverage yo 99.9%

0eda98e

Removed the break statement to improve code coverage.

aed3358

vil02 requested changes Jun 24, 2024

View reviewed changes

JAPNITSINGH added 3 commits June 25, 2024 17:09

Removed comments and used SortUtils

ab905bf

Merge branch 'TheAlgorithms:master' into Branch_Smooth_Sort

65ca159

Merge branch 'Branch_Smooth_Sort' of github.com:JAPNITSINGH/Java into…

d5e3b95

… Branch_Smooth_Sort

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Smooth Sort #5236

Implement Smooth Sort #5236

JAPNITSINGH commented Jun 18, 2024

JAPNITSINGH commented Jun 18, 2024

vil02 Jun 18, 2024

vil02 Jun 18, 2024

JAPNITSINGH Jun 19, 2024

JAPNITSINGH Jun 22, 2024 •

edited

Loading

JAPNITSINGH Jun 24, 2024

codecov-commenter commented Jun 24, 2024 •

edited

Loading

vil02 left a comment

vil02 Jun 24, 2024

JAPNITSINGH Jun 25, 2024

JAPNITSINGH Jun 25, 2024 •

edited

Loading

vil02 Jun 24, 2024

JAPNITSINGH Jun 25, 2024

JAPNITSINGH Jun 25, 2024 •

edited

Loading

vil02 Jun 24, 2024

vil02 Jun 24, 2024

vil02 Jun 24, 2024

vil02 Jun 24, 2024

JAPNITSINGH Jun 25, 2024

vil02 Jun 24, 2024

vil02 Jun 24, 2024

JAPNITSINGH Jun 25, 2024 •

edited

Loading

JAPNITSINGH Jun 25, 2024 •

edited

Loading

vil02 Jun 24, 2024

JAPNITSINGH Jun 25, 2024

vil02 Jun 24, 2024

JAPNITSINGH Jun 25, 2024

JAPNITSINGH commented Jun 25, 2024 •

edited

Loading

JAPNITSINGH commented Jul 3, 2024

		Integer[] leonardoNumbers = {1, 1, 3, 5, 9, 15, 25, 41, 67, 109, 177, 287, 465, 753, 1219, 1973, 3193, 5167, 8361, 13529, 21891, 35421, 57313, 92735, 150049, 242785, 392835, 635621, 1028457, 1664079, 2692537, 4356617, 7049155, 1405773, 18454929, 29860703, 48315633, 78176337, 126491971,
		204668309, 331160281, 535828591};

	swap(array, prevRootNodeIndex, currentRootNodeIndex);
	SortUtils.swap(array, prevRootNodeIndex, currentRootNodeIndex);

		// swap
		swap(array, prevRootNodeIndex, currentRootNodeIndex);

		// maxHeapifyLeonardoTree(rightChildIndex, currentLeonardoLevel - 2, array);
		// maxHeapifyLeonardoTree(leftChildIndex, currentLeonardoLevel - 1, array);

Implement Smooth Sort #5236

Are you sure you want to change the base?

Implement Smooth Sort #5236

Conversation

JAPNITSINGH commented Jun 18, 2024

JAPNITSINGH commented Jun 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JAPNITSINGH Jun 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 24, 2024 • edited Loading

Codecov Report

vil02 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JAPNITSINGH Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JAPNITSINGH Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JAPNITSINGH Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

JAPNITSINGH Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JAPNITSINGH commented Jun 25, 2024 • edited Loading

JAPNITSINGH commented Jul 3, 2024

JAPNITSINGH Jun 22, 2024 •

edited

Loading

codecov-commenter commented Jun 24, 2024 •

edited

Loading

JAPNITSINGH Jun 25, 2024 •

edited

Loading

JAPNITSINGH Jun 25, 2024 •

edited

Loading

JAPNITSINGH Jun 25, 2024 •

edited

Loading

JAPNITSINGH Jun 25, 2024 •

edited

Loading

JAPNITSINGH commented Jun 25, 2024 •

edited

Loading