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

Regular sequences: implement convolution / ring structure #35894

Merged
merged 16 commits into from Aug 5, 2023

Conversation

dkrenn
Copy link
Contributor

@dkrenn dkrenn commented Jul 4, 2023

📚 Description

Add/implement convolution of two regular sequences, therefore making the parent a ring.

Details of the algorithm: convolution-sagemath-pr-35894.pdf

See also meta issue #21202.

📝 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

No formal dependencies, but there are trivial merge conflicts---in both branches, methods are inserted at the same position in the code---between:

… 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
  ...
@dkrenn dkrenn marked this pull request as ready for review July 4, 2023 18:16
Copy link
Contributor

@cheuberg cheuberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are my comments on this PR.

src/sage/combinat/k_regular_sequence.py Show resolved Hide resolved
src/sage/combinat/k_regular_sequence.py Outdated Show resolved Hide resolved
src/sage/combinat/k_regular_sequence.py Outdated Show resolved Hide resolved
src/sage/combinat/recognizable_series.py Outdated Show resolved Hide resolved
src/sage/combinat/recognizable_series.py Outdated Show resolved Hide resolved
src/sage/combinat/recognizable_series.py Outdated Show resolved Hide resolved
dkrenn and others added 5 commits July 25, 2023 09:02
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)
vbraun pushed a commit that referenced this pull request Jul 30, 2023
    
### 📚 Description

It deals with the situation when mu[0]*right != right in k-regular
sequences.

Fixes #21343. This PR is created from the branch/code that has been on
the corresponding trac ticket + merging in a current SageMath version.

See also meta issue #21202.

### 📝 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

- #21204: guessing k-regular sequences from the first few values

No formal dependencies, but there are trivial merge conflicts---in both
branches, methods are inserted at the same position in the code---
between:
- #21343: deal with mu[0]*right != right in k-regular sequences
- #35894: Regular sequences: implement convolution / ring structure
    
URL: #35896
Reported by: Daniel Krenn
Reviewer(s): cheuberg, Daniel Krenn
* 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
  ...
@dkrenn
Copy link
Contributor Author

dkrenn commented Jul 31, 2023

Current develop (10.1.beta8) merged to avoid merge conflicts.

Copy link
Contributor

@cheuberg cheuberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please set to positive once the PDF is uploaded.

@github-actions
Copy link

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

@vbraun vbraun merged commit edc4c57 into sagemath:develop Aug 5, 2023
16 of 18 checks passed
vbraun pushed a commit that referenced this pull request Aug 5, 2023
…uence module

    
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 #35894).
- Remove the experimental warning.

### 📝 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

- #35894
    
URL: #36011
Reported by: Daniel Krenn
Reviewer(s): cheuberg, Daniel Krenn
@mkoeppe mkoeppe added this to the sage-10.1 milestone Aug 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants