Regular sequence: closing experimental phase of regular sequence module #36011

dkrenn · 2023-07-31T16:50:10Z

This PR closes the experimental-phase of the regular sequences module. This module has been experimental für a very long time now. Lately #35894 has been completed so this is now a fully-functional ring/algebra. Except for the consequences of #35894 (renaming; see below), the interface is stable for very long time.

This PR does:

Rename to RegularSequenceRing (as this structure now is a ring due to Regular sequences: implement convolution / ring structure #35894).
Remove the experimental warning.

📝 Checklist

The title is concise, informative, and self-explanatory.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation accordingly.

⌛ Dependencies

Regular sequences: implement convolution / ring structure #35894

… regular-sequences-mul * origin/u/dkrenn/k-regular-warning: (69 commits) fix doctests after merge test RuntimeError no invertible matrix add comment and ref to sagemath#35748 remove W293 blank line contains whitespace fix docstring (LaTeX) fix whitespaces better handling of n_max add another test add another test fixup n_max fix left/right issue when bootstraping guessing more consistent naming in DEBUG output consequently rename to linear_combination simplify code by removing variables make NoLinearCombination a RuntimeError remove unneeded import Apply suggestions from code review Apply suggestions from code review write error message rename, describe and test max_exponent ...

Co-authored-by: cheuberg <clemens.heuberger@aau.at>

…ul' into regular-sequences-mul * refs/remotes/origin/regular-sequences-mul: Apply suggestions from code review

* u/dkrenn/k-regular-warning: Fix one typo Reference [HKL2021] appeared (and is now [HKL2022]) use HKL2021-algorithm in .regenerated fix broken links in docstrings simplify representative example (pascal triangle) extend doctest: show linear representation Apply suggestions from code review Update src/sage/combinat/k_regular_sequence.py fix left/right vector sequence issue in docstrings implement coefficient_of_n (for __getitem__) break long line in doctest rewrite minimiz_result decorator: deal with result is self use .linear_representation in doctests everywhere minor cleanup/readability change in .regenerated k-regular sequences: refactor .regenerated (use minimize_result decorator) fix doctest magic comments (according to relint)

* upstream/develop: (1372 commits) Updated SageMath version to 10.1.beta8 remove multiple call of Vmatrix and Vmodule remove PGE and some listcomp get two entry directly tests passed Still permutation wip fix one issue fix fix typo store inverse of permutation Direct_Permute WIP PermutedMatrixWindow redundant line Style fixes PR sagemath#35891: fix issue in src/sage/geometry/polyhedral_complex.py fix merging problems add type checks for parameters PR sagemath#35956: fix typo PR sagemath#35956: fix doctests fix another issue ...

…erimental * regular-sequences-mul: algorithm block break line in doctest make description of method "one" more precise Apply suggestions from code review convolution: rename variables k-regular sequences: convolution/docs k-regular sequences: extend category to Algebra k-regular sequences: implement convolution k-regular sequences: fix .one() cleanup old, unreadable dict creations recognizable series: embed coefficient ring recognizable series: properly deal with zero

dkrenn · 2023-07-31T16:51:09Z

CC @cheuberg

cheuberg

The changes so far are fine with me, but I'd go farther with removing the name k of the base. I started making suggestions but then stopped because a search-and-replace for "k-regular" seems to be more efficient.

cheuberg · 2023-07-31T19:10:01Z

src/sage/combinat/all.py

@@ -277,7 +277,7 @@
 lazy_import('sage.combinat.binary_recurrence_sequences',
            'BinaryRecurrenceSequence')
 lazy_import('sage.combinat.recognizable_series', 'RecognizableSeriesSpace')
-lazy_import('sage.combinat.k_regular_sequence', 'kRegularSequenceSpace')
+lazy_import('sage.combinat.k_regular_sequence', 'RegularSequenceRing')


I propose to rename the file k_regular_sequence.py to regular_sequence.py (after merging #36001) for consistency: now that everything is renamed in such a way to remove the variable k, that would seem to be a logical move to me.

Yes, indeed, I agree that this seems a logical move. I prepared a commit and will push it soon. (BTW it does not seem to be a problem due to #36001 when merging; git appearently takes care of this.)

cheuberg · 2023-07-31T19:10:55Z

src/sage/combinat/k_regular_sequence.py

@@ -6,21 +6,6 @@
 found, for example, on the :wikipedia:`k-regular_sequence` or in


The k in lines 3 and 5 could be removed.

Not convinced; see below (#36011 (comment))

src/sage/combinat/k_regular_sequence.py

dkrenn · 2023-08-01T08:29:53Z

The changes so far are fine with me, but I'd go farther with removing the name k of the base.

At the moment, I am not fully convinced that this is the way to go. After all, the used term is "k-regular sequence"; see titles of the corresponding articles and also the Wikipedia article https://en.wikipedia.org/wiki/K-regular_sequence consistently uses this term including the base. (FWIW The wikipedia article https://en.wikipedia.org/wiki/Regular_sequence refers to a different concept, but at least in the first line k-regular sequences are referenced; as our file is in sage.combinat, I do not see a problem with the renaming of the file as discussed above.) Moreover, I do not see a problem that the class RegularSequence represents a k-regular sequence because from a coding perspective it seems clear that the parameter k has to be fixed somewhere (but not in the name of the class).

I, therefore, have some preference, for keeping this as it is.

In case we agree to change, I have a strong preference for keeping the title (line 3) of the module as it is (i.e. including the base) and think we should give both names k-regular and regular (and say something that we will simply skip the base) in the paragraph starting with line 5.

github-actions · 2023-08-01T10:25:39Z

Documentation preview for this PR (built with commit cd04076; changes) is ready! 🎉

cheuberg

Changes are fine with me. The question on how to call the sequences in the documentation is perhaps not very urgent (and independent of removing the experimental notice).

Two comments:

while the Wikipedia entry for regular sequences is called "k-regular sequence", the entry there for the closely related automatic sequence is called "automatic sequence" with an explanation there that it is sometimes called a "k-automatic sequence".
concerning the documentation here: for some methods, mentioning "k" makes more sense to me than for others. E.g. for the constructor of a regular sequence, it makes more sense to me to say that it is "k-recursive" because the description of the family mu refers to the same k. However, for the oneline description of backwards difference, the "k" seems to be quite redundant to me: it does not occur anywhere else in the description of the method.

But, as I said, we can also merge it essentially as it is, perhaps just adding a short sentence at the very beginning.

dkrenn · 2023-08-02T13:11:49Z

Changes are fine with me. The question on how to call the sequences in the documentation is perhaps not very urgent (and independent of removing the experimental notice).

This is now #36019.

sagemathgh-36029: k-regular sequences: boundedness   Implements an algorithm to check for boundedness of k-regular sequences. See issue sagemath#22964. (Coding and reviewing already happend on trac (see sagemath#22964) and the branch was ready but never merged due to a trivial issue. Now the current develop (10.1.beta8) has been merged in to avoid conflicts the upcoming sagemath#36011 (where a renaming took place; set as depedency now). ### 📝 Checklist - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies - sagemath#36011: Experimental warning removed in class used in this PR and class was renamed. URL: sagemath#36029 Reported by: Daniel Krenn Reviewer(s): cheuberg, Daniel Krenn, Gabriel Lipnik

dkrenn and others added 20 commits July 4, 2023 07:59

recognizable series: properly deal with zero

864d842

recognizable series: embed coefficient ring

37a1b2a

cleanup old, unreadable dict creations

32e6b77

k-regular sequences: fix .one()

a57f92d

k-regular sequences: implement convolution

ceb3c79

k-regular sequences: extend category to Algebra

bb3d701

k-regular sequences: convolution/docs

0710136

convolution: rename variables

91b4fc6

Apply suggestions from code review

90d347d

Co-authored-by: cheuberg <clemens.heuberger@aau.at>

Merge remote-tracking branch 'refs/remotes/origin/regular-sequences-m…

420993c

…ul' into regular-sequences-mul * refs/remotes/origin/regular-sequences-mul: Apply suggestions from code review

make description of method "one" more precise

4ed1e49

break line in doctest

133374e

algorithm block

05de6cc

remove experimental warning

15de625

rename to RegularSequenceRing

edf6298

rename to RegularSequence

c079ada

dkrenn added p: major / 3 s: needs review c: combinatorics labels Jul 31, 2023

cheuberg suggested changes Jul 31, 2023

View reviewed changes

dkrenn added 3 commits August 1, 2023 09:40

rename to src/sage/combinat/regular_sequence.py

88f0ba4

rename follow-up: adapt new path in sources

4c7c29c

consistent title of module

cd04076

cheuberg approved these changes Aug 2, 2023

View reviewed changes

dkrenn added s: positive review and removed s: needs review labels Aug 2, 2023

dkrenn mentioned this pull request Aug 2, 2023

unify notation k-regular vs regular sequence in docstrings in corresponding module #36019

Open

2 tasks

dkrenn added a commit to dkrenn/sage that referenced this pull request Aug 3, 2023

rename files (in preparation for sagemath#36011)

c2f4fde

dkrenn added a commit to dkrenn/sage that referenced this pull request Aug 3, 2023

adapt from changes in sagemath#36011

92cc4d1

dkrenn mentioned this pull request Aug 3, 2023

k-regular sequences: boundedness #36029

Merged

5 tasks

dkrenn added a commit to dkrenn/sage that referenced this pull request Aug 3, 2023

adapt from changes in sagemath#36011

40d001f

vbraun merged commit 1329316 into sagemath:develop Aug 5, 2023

github-actions bot removed the s: positive review label Aug 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Regular sequence: closing experimental phase of regular sequence module #36011

Regular sequence: closing experimental phase of regular sequence module #36011

Uh oh!

dkrenn commented Jul 31, 2023

Uh oh!

dkrenn commented Jul 31, 2023

Uh oh!

cheuberg left a comment

Uh oh!

cheuberg Jul 31, 2023

Uh oh!

dkrenn Aug 1, 2023

Uh oh!

cheuberg Jul 31, 2023

Uh oh!

dkrenn Aug 1, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dkrenn commented Aug 1, 2023

Uh oh!

github-actions bot commented Aug 1, 2023

Uh oh!

cheuberg left a comment

Uh oh!

dkrenn commented Aug 2, 2023

Uh oh!

Uh oh!

		@@ -6,21 +6,6 @@
		found, for example, on the :wikipedia:`k-regular_sequence` or in

Uh oh!

Regular sequence: closing experimental phase of regular sequence module #36011

Regular sequence: closing experimental phase of regular sequence module #36011

Uh oh!

Conversation

dkrenn commented Jul 31, 2023

📝 Checklist

⌛ Dependencies

Uh oh!

dkrenn commented Jul 31, 2023

Uh oh!

cheuberg left a comment

Choose a reason for hiding this comment

Uh oh!

cheuberg Jul 31, 2023

Choose a reason for hiding this comment

Uh oh!

dkrenn Aug 1, 2023

Choose a reason for hiding this comment

Uh oh!

cheuberg Jul 31, 2023

Choose a reason for hiding this comment

Uh oh!

dkrenn Aug 1, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dkrenn commented Aug 1, 2023

Uh oh!

github-actions bot commented Aug 1, 2023

Uh oh!

cheuberg left a comment

Choose a reason for hiding this comment

Uh oh!

dkrenn commented Aug 2, 2023

Uh oh!

Uh oh!