Skip to content

Implemented longest increasing subsequence dp algorithm #19

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

Merged
merged 12 commits into from
Jan 3, 2025

Conversation

spirosmaggioros
Copy link
Contributor

@spirosmaggioros spirosmaggioros commented Jan 3, 2025

Just an lis implementation using the lower bound trick that runs in O(nlogn)

kassane
kassane previously approved these changes Jan 3, 2025
@kassane
Copy link
Member

kassane commented Jan 3, 2025

Nice implementation, @spirosmaggioros.

A question: Why is there always need ArrayList? BoundedArray couldn't have been applied?

References

@spirosmaggioros
Copy link
Contributor Author

spirosmaggioros commented Jan 3, 2025

Nice implementation, @spirosmaggioros.

A question: Why is there always need ArrayList? BoundedArray couldn't have been applied?

References

Haven't looked it up, i learned zig the past 4 days. One day i saw that the std::vector equivelant is the ArrayList, so i thought i can just use this. I can look it up and fix it if you wish or if you think it will be better in any case.

Well, looked it up. I can't use a bounded array because i can't have a fixed size. The size needs to be dynamic as i can't know somehow with how many elements i will end up. I guess setting an upper bound is not ideal and will be less efficient.

@kassane

This comment was marked as outdated.

@spirosmaggioros
Copy link
Contributor Author

spirosmaggioros commented Jan 3, 2025

My diff patch for suggestion

diff --git a/dynamicProgramming/longestIncreasingSubsequence.zig b/dynamicProgramming/longestIncreasingSubsequence.zig
index 243c1e3..c1a609f 100644
--- a/dynamicProgramming/longestIncreasingSubsequence.zig
+++ b/dynamicProgramming/longestIncreasingSubsequence.zig
@@ -1,7 +1,7 @@
 const std = @import("std");
-const print = std.debug.print;
 const testing = std.testing;
-const ArrayList = std.ArrayList;
+const BoundedArray = std.BoundedArray;
+const panic = std.debug.panic;
 
 // Function that returns the lower bound in O(logn)
 pub fn lower_bound(arr: []const i32, key: i32) usize {
@@ -26,42 +26,38 @@ pub fn lower_bound(arr: []const i32, key: i32) usize {
 
 // Function that returns the length of the longest increasing subsequence of an array
 // Runs in O(nlogn) using the lower bound function
-pub fn lis(arr: []const i32) usize {
-    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
-    defer _ = gpa.deinit();
-
-    const allocator = gpa.allocator();
-
-    var v = ArrayList(i32).init(allocator);
-    defer v.deinit();
-
-    const n = arr.len;
-
-    for (0..n) |i| {
-        const it = lower_bound(v.items, arr[i]);
-        if (it == v.items.len) {
-            _ = v.append(arr[i]) catch return 0;
+pub fn lis(arr: []const i32, comptime n: usize) usize {
+    var v = BoundedArray(i32, n).init(0) catch |err| {
+        panic("Error: {}\n", .{err});
+    };
+
+    for (arr) |value| {
+        const it = lower_bound(v.slice(), value);
+        if (it == v.len) {
+            v.append(value) catch |err| {
+                panic("Error: {}\n", .{err});
+            };
         } else {
-            v.items[it] = arr[i];
+            v.set(it, value);
         }
     }
 
-    return v.items.len;
+    return v.len;
 }
 
 test "testing longest increasing subsequence function" {
     const v = [4]i32{ 1, 5, 6, 7 };
-    try testing.expect(lis(&v) == 4);
+    try testing.expect(lis(&v, v.len) == 4);
 
     const v2 = [5]i32{ 1, -1, 5, 6, 7 };
-    try testing.expect(lis(&v2) == 4);
+    try testing.expect(lis(&v2, v2.len) == 4);
 
     const v3 = [5]i32{ 1, 2, -1, 0, 1 };
-    try testing.expect(lis(&v3) == 3);
+    try testing.expect(lis(&v3, v3.len) == 3);
 
-    const v4 = [0]i32{};
-    try testing.expect(lis(&v4) == 0);
+    const v4: [0]i32 = .{};
+    try testing.expect(lis(&v4, v4.len + 1) == 0);
 
-    const v5 = [5]i32{ 0, 0, 0, 0, 0 };
-    try testing.expect(lis(&v5) == 1);
+    const v5 = std.mem.zeroes([5]i32);
+    try testing.expect(lis(&v5, v5.len) == 1);
 }

Note

Whats v4.len + 1? zig v0.13.0 [0]T{} [+1 inc fix] get error

/home/kassane/zig/0.13.0/files/lib/std/bounded_array.zig:114:25: error: type 'u0' cannot represent integer value '1'
           self.len += 1;
                       ^

but, v0.14.0-dev no error

That's your implementation, i don't pass any length in my original one. I think you meant to write v4.len.

As for your implementation, if you have an array of size 1000 and the lis is 2, the v array inside the lis function will only grow up to len = 2. So, actually setting the size - thus allocating - to 1000 won't be efficient. That's just my view.

@kassane
Copy link
Member

kassane commented Jan 3, 2025

think you meant to write v4.len.

v4.len get error with [0]T{} in v0.13.0 (bound_Array only).
Keeping arraylist, make possible add std.mem.Allocator on fn lis to user set allocator choiced.

@spirosmaggioros
Copy link
Contributor Author

think you meant to write v4.len.

v4.len get error with [0]T{} in v0.13.0 (bound_Array only). Keeping arraylist, make possible add std.mem.Allocator on fn lis to user set allocator choiced.

will do

@kassane kassane merged commit ce4cb17 into TheAlgorithms:main Jan 3, 2025
5 checks passed
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.

2 participants