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

Implement longest_increasing_subsequence_number #34218

Closed
videlec opened this issue Jul 25, 2022 · 20 comments
Closed

Implement longest_increasing_subsequence_number #34218

videlec opened this issue Jul 25, 2022 · 20 comments

Comments

@videlec
Copy link
Contributor

videlec commented Jul 25, 2022

Following the method of #31451, we implement a method that returns the number of maximal increasing subsequences of a permutation. This method is much faster than listing them all

sage: %time _ = sum(len(p.longest_increasing_subsequences()) for p in Permutations(8))
CPU times: user 1.76 s, sys: 2.97 ms, total: 1.76 s
Wall time: 1.77 s
120770
sage: %time sum(p.longest_increasing_subsequences_number() for p in Permutations(8))
CPU times: user 328 ms, sys: 0 ns, total: 328 ms
Wall time: 328 ms
120770

Depends on #31451
Depends on #34214

CC: @nadialafreniere @dcoudert

Component: combinatorics

Author: Nadia Lafrenière, Vincent Delecroix

Branch/Commit: 11e2592

Reviewer: David Coudert

Issue created by migration from https://trac.sagemath.org/ticket/34218

@videlec videlec added this to the sage-9.7 milestone Jul 25, 2022
@nadialafreniere
Copy link
Contributor

@nadialafreniere
Copy link
Contributor

Commit: cee005d

@nadialafreniere
Copy link
Contributor

comment:2

I tried implementing it, but I'm not convinced by the result. The method with the adjacency matrix of the digraph seems to be slower. Mainly, I think that a lot of the problem comes from taking the exponential of the adjacency matrix of the digraph (an (n+2)-by-(n+2) matrix raised to a power that is in θ(√n) in average), and this seems to slow the process down much more than listing the longest increasing subsequences.

After coding it, I got the following times:

sage: %timeit len(Permutations(100).random_element().longest_increasing_subsequences())  # Naive 
2.8 ms ± 284 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
sage: %timeit Permutations(100).random_element().longest_increasing_subsequences_number()  # new code
4.42 ms ± 42 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

Even though I'm uploading my code to the trac, I think we might want to give up on the project. Or, if you know of a way to make it efficient, I would love to see it.


Last 10 new commits:

98030ffImplementation of digraph strategy for Longest Increasing Subsequences
8d1c1d6Fixing a bug with n=0 for longest increasing subsequences
62b9a39Simplified end of code for longest increasing subsequences
4a7f5c8Use of bisect for columns in longest_increasing_subsequences
5969edaproper formatting for longest_increasing_subsequences
a5c59f3Merge branch 't/31451/LongestIncreasingSubsequences' into t/34218/implement_longest_increasing_subsequence_number
2e183cftrac #34214: faster longest_increasing_subsequence_length
15aa550trac #34214: remove a useless variable
c897f83Merge branch 't/34214/public/combinat/34214' into t/34218/implement_longest_increasing_subsequence_number
cee005dImplemented longest_increasing_subsequences_number()

@videlec
Copy link
Contributor Author

videlec commented Jul 25, 2022

comment:3

Indeed, matrix powering is not clever enough. Though one can avoid the creation of the digraph which is time consuming.


New commits:

9b946a6faster p.longest_increasing_subsequeces_number

@videlec
Copy link
Contributor Author

videlec commented Jul 25, 2022

@videlec
Copy link
Contributor Author

videlec commented Jul 25, 2022

Author: Nadia Lafrenière, Vincent Delecroix

@videlec
Copy link
Contributor Author

videlec commented Jul 25, 2022

Changed commit from cee005d to 9b946a6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 25, 2022

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a98a60ffaster p.longest_increasing_subsequences_number

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 25, 2022

Changed commit from 9b946a6 to a98a60f

@videlec
Copy link
Contributor Author

videlec commented Jul 25, 2022

comment:5

At commit a98a60f I got

sage: %time sum(len(p.longest_increasing_subsequences()) for p in Permutations(8))
CPU times: user 1.76 s, sys: 2.97 ms, total: 1.76 s
Wall time: 1.77 s
120770
sage: %time sum(p.longest_increasing_subsequences_number() for p in Permutations(8))
CPU times: user 328 ms, sys: 0 ns, total: 328 ms
Wall time: 328 ms
120770

@videlec
Copy link
Contributor Author

videlec commented Jul 25, 2022

comment:6

And same test as your comment:2 gives on my machine

sage: %timeit len(Permutations(100).random_element().longest_increasing_subsequences())  # Naive 
1.55 ms ± 69.8 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
sage: %timeit Permutations(100).random_element().longest_increasing_subsequences_number()  # new code
122 µs ± 2.38 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

@videlec

This comment has been minimized.

@dcoudert
Copy link
Contributor

comment:9

I was about to explain how to count the number of paths on a DAG when all arcs go from level i to level i+1 in time O(|V| + |E|), but your solution avoids the creation of the DAG. This is smart.

You could add a little explanation of the algorithm, at least for Nadia.

For the documentation, you could use :meth:~.longest_increasing_subsequences, or something like that.

Otherwise, LGTM.

@dcoudert
Copy link
Contributor

comment:10

For Nadia: roughly, in the DAG, the number of paths to reach vertex x of column i+1 is the sum over the predecessors of x (all in column i) of the number of paths from to source to each of these predecessors. For initialization, there is a single path to go from the source to a vertex in column 0 (a successor of the source). then, you iterate over the columns and you add the count of x in the current column to each of its successors.

The method of Vincent do the same computation but avoids to build the DAG.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 26, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

88676d8optimization and typing
11e2592more documentation

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 26, 2022

Changed commit from a98a60f to 11e2592

@dcoudert
Copy link
Contributor

comment:12

LGTM.

@dcoudert
Copy link
Contributor

Reviewer: David Coudert

@nadialafreniere
Copy link
Contributor

comment:13

Replying to @dcoudert:

For Nadia: roughly, in the DAG, the number of paths to reach vertex x of column i+1 is the sum over the predecessors of x (all in column i) of the number of paths from to source to each of these predecessors. For initialization, there is a single path to go from the source to a vertex in column 0 (a successor of the source). then, you iterate over the columns and you add the count of x in the current column to each of its successors.

The method of Vincent do the same computation but avoids to build the DAG.

Nice job! This looks very good!

@vbraun
Copy link
Member

vbraun commented Aug 1, 2022

Changed branch from public/34218 to 11e2592

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

No branches or pull requests

4 participants