sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.47k stars 485 forks source link

provide a class for partitions with bounded length and minimal part #38904

Open mantepse opened 3 weeks ago

mantepse commented 3 weeks ago

We provide a new class, PartitionsSmallestGE to handle subsets of the constraints length, min_length, max_length and min_part, to make cardinality computations with such constraints reasonable.

Fixes #38897

github-actions[bot] commented 3 weeks ago

Documentation preview for this PR (built with commit 4f42af7c142f7446025f47c4b0dbc2ac8146063b; changes) is ready! :tada: This preview will update shortly after each push to this PR.

mantepse commented 1 week ago

@fchapoton, could you please have a look so it doesn't rot away?

mantepse commented 1 week ago

Thank you! Sorry, I couldn't resist.

mantepse commented 1 week ago

Dear @fchapoton, I would like to put this back to "needs work", because it doesn't quite solve Max' issue, and it's not hard to do that, but I'd like to avoid deprecation of something I just introduced.

Is this OK with you or would you prefer if I open a new pull request?

Ideally, I think there should be a single class for partitions of n with parts in a given set or interval and length in a given interval, which computes the cardinality efficiently using a few case distinctions and a recurrence if necessary.

mantepse commented 6 days ago

This is now much better. Almost all the time is now spent in IntegerListsLex.__init__, which is a pity, but can probably not be fixed in this pull request.

I'm open for a better name for the class Partitions_parts_length_restricted. I'm also open for deprecating PartitionsGreatestLE.

mantepse commented 6 days ago

I find it a bit frustrating that we do not need the IntegerListsLex.__init__ at all to compute the cardinality. It would be nice to have a pattern where such an expensive initialisation is postponed until it's needed.

Failing that, it seems that most of the time is spent in Envelope.__init__. Maybe this can be optimized.

mantepse commented 5 days ago

The solution was very simple, so I implemented it.

Max' problem is still much better solved without using Partitions (it seems to take roughly twice as long), because Partitions has UniqueRepresentation, which is expensive if we call it with very many different arguments - we are creating an entry in the cache not only for each size, but for each restriction.

mantepse commented 5 days ago

@tscrim, there seems to be a problem with FockSpace. In FockSpaceTruncated.F.__init__, the index set is set to be partitions of F._n, but that doesn't seem to be quite true. Could you please check?

https://github.com/sagemath/sage/blob/39ebbe45ca5cdf87d8ae23a28b3f784130be3d1a/src/sage/algebras/quantum_groups/fock_space.py#L1734-L1759

tscrim commented 4 days ago

@tscrim, there seems to be a problem with FockSpace. In FockSpaceTruncated.F.__init__, the index set is set to be partitions of F._n, but that doesn't seem to be quite true. Could you please check?

Yes, that is not correct. It should be over all partitions of length \leq k (as per the doc of FockSpaceTruncated).

mantepse commented 3 days ago

OK, I made a catch-all class for partitions which are not restricted by size. This can easily be improved, but I'd prefer to postpone this.

mantepse commented 3 days ago

@fchapoton, is the lint error in error?

Other than that, I think that the pull request should be ready to go.

fchapoton commented 3 days ago

somebody should add PLC0206 to the ruff-minimal ignore list in another pull request

DONE; please review #39021