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

basic element and parent for k-regular sequences #21203

Closed
dkrenn opened this issue Aug 10, 2016 · 60 comments
Closed

basic element and parent for k-regular sequences #21203

dkrenn opened this issue Aug 10, 2016 · 60 comments

Comments

@dkrenn
Copy link
Contributor

dkrenn commented Aug 10, 2016

Implement minimal element and parent classes for working with k-regular sequences

See also Meta ticket #21202.

Depends on #21164
Depends on #21180
Depends on #21200
Depends on #21295

CC: @rwst @galipnik

Component: combinatorics

Author: Daniel Krenn

Branch/Commit: d532b56

Reviewer: Clemens Heuberger

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

@dkrenn
Copy link
Contributor Author

dkrenn commented Aug 10, 2016

Branch: u/dkrenn/sequences/k-regular

@dkrenn
Copy link
Contributor Author

dkrenn commented Aug 10, 2016

Dependencies: #21164, #21180, #21200

@dkrenn
Copy link
Contributor Author

dkrenn commented Aug 10, 2016

Commit: 9c64180

@dkrenn
Copy link
Contributor Author

dkrenn commented Aug 10, 2016

Last 10 new commits:

965ec0fMerge branch 'u/dkrenn/lazy_list-unbound-formatter' into u/dkrenn/sequences/k-regular
d8ae454make product_of_matrices a private method
03c6418docstring of _product_of_matrices_
16e041bphrase error message and test it
091a276use lazy_list_formatter in _repr_
e233dfdwrite the missing docstrings
1818628modify docstring at top of file
1e5efc8rename to kRegularSequenceSpace
c14eceb__classcall__ and __init__
9c64180write remaining docstrings

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 11, 2016

Changed commit from 9c64180 to b92ff44

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 11, 2016

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

d3e2df6Trac #21200: simplify requirements for input (only iterable needed)
7e166c3Merge branch 'u/dkrenn/lazy_list-unbound-formatter' into u/dkrenn/sequences/k-regular
4102e47__iter__ method
b92ff44simplify `_repr_` code

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 18, 2016

Changed commit from b92ff44 to b4d5590

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 18, 2016

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

a2c8e91new repr
089a0earemove output_function
6b67851adapt docstring of transposed
800a329Merge branch 'u/dkrenn/sequences/recognizable' into u/dkrenn/sequences/k-regular
28149c9import of recognizable series
a26a051matrices --> mu
9128d28initial, selection --> left, right
5cf8f56split `__normalize__` from `__classcall__` (to deal better with inheritance)
4bc0749Merge branch 'u/dkrenn/sequences/recognizable' into u/dkrenn/sequences/k-regular
b4d5590use new code for RecognizableSeries

@dkrenn
Copy link
Contributor Author

dkrenn commented Aug 19, 2016

Changed dependencies from #21164, #21180, #21200 to #21164, #21180, #21200, #21295

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 19, 2016

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

5f06dacminor rewrite in _repr_
b18d3aacheck input of RecognizableSequenceSpace and document it
85c1a03checks for creating recognizable series
eec776efix doctests
7d96697coverage to 100%
4a1db0ereorder imports
62ebcdacomplete documentation
4331957Merge branch 'u/dkrenn/sequences/recognizable' into u/dkrenn/sequences/k-regular
dab4db8complete docstrings and 100% coverage
9afc4accross-references

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 19, 2016

Changed commit from b4d5590 to 9afc4ac

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 19, 2016

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

fd4f541move "transpose" in docstring to correct place
4eb1544Merge branch 'u/dkrenn/sequences/recognizable' into u/dkrenn/sequences/k-regular
cf1b8cccorrect one link

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 19, 2016

Changed commit from 9afc4ac to cf1b8cc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 22, 2016

Changed commit from cf1b8cc to 71aef7f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 22, 2016

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

863d57aremove None-sense to simplify code
aec95ceextend element constructor
9e12383add experimental warning
69289feMerge branch 'u/dkrenn/sequences/recognizable' into u/dkrenn/sequences/k-regular
71aef7ffix doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 23, 2016

Changed commit from 71aef7f to 5919120

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 23, 2016

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

41874bafix a typo
0f538a9rearrange __getitem__
7495635Merge branch 'u/dkrenn/sequences/recognizable' into u/dkrenn/sequences/k-regular
e53b906update `__getitem__` to follow recognizable series
1794766flag to skip multiplication with left/right
b060652minor rewrite in minimized
31e2fcfMerge branch 'u/dkrenn/sequences/recognizable' into u/dkrenn/sequences/k-regular
893bd5cexperimental warning in k_regular_sequence
189ce50pass keywords to coefficient_of_word
5919120doctesting all parameters in coefficient_of_word

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 25, 2016

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

c7243a6Merge tag '7.4.beta1' into t/21295/sequences/recognizable
4012778Merge branch 't/21295/sequences/recognizable' into t/21203/sequences/k-regular

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 25, 2016

Changed commit from 5919120 to 4012778

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 25, 2016

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

5dbedabfix doctests (experimental warning)
143ea73fix building of docs (no idea why it failed)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 25, 2016

Changed commit from 4012778 to 143ea73

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 26, 2016

Changed commit from 143ea73 to d5c378d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 6, 2021

Changed commit from 9e82a65 to c4bf7ae

@dkrenn
Copy link
Contributor Author

dkrenn commented May 6, 2021

comment:35

Replying to @cheuberg:

  1. One more very minor detail: docstring of __getitem__: Apparently `n`th is not recognised correctly: there is an error message "docstring of sage.combinat.k_regular_sequence.kRegularSequence.__getitem__:1: WARNING: Inline interpreted text or phrase reference start-string without end-string." when running ./sage --docbuild --underscore reference/combinat html, and no LaTeX formatting occurs.

Fixed

@dkrenn
Copy link
Contributor Author

dkrenn commented May 6, 2021

comment:36

Replying to @cheuberg:

  1. Example Binary sum of digits: It would be helpful to explain why this is a linear representation of the binary sum of digits. (However: why not choosing a simpler representation having the sum of digits and a constant sequence in the vector? Here, the first component is one minus the sum of digits)

Example changed (easier representation used) and more explanations.

  1. Example Number of odd entries in Pascal's triangle: u(n) seems to be the number of odd entries up to row n-1. This could be made more explicit and the linear representation as well as the recursive formula could be explained, possibly with a reference.

Linear representation changed and some text added. For sure more could always be made...

  1. kRegularSequence: please add an explanation on how to access elements of the sequence to the documentation of the class (it is now in the documentation of the module, but without explanation, and the docstrings of __getitem__ and __iter__ are not shown in the documentation.

Example added.

  1. kRegularSequenceSpace: I think that to the casual user, the use of the word "coefficient" might be surprising.

Yes, indeed. Naming is not that easy here. Everything with "base" seems to be no option was we have "k" that is called base as well. I now give "coefficient_ring" a try (also changed at at #21295), but I am very open to other, better suggestions.

  1. _n_to_index_: I would prefer a ValueError with meaningful message instead of an OverflowError.

Changed.

@cheuberg
Copy link
Contributor

cheuberg commented May 6, 2021

comment:38

Thank you for your changes.

I have one question and another minor suggestion.

  1. What was the motivation to remove "If not specified (None), then the integer ring is used." ?

  2. I suggest to use raise ... from None where appropriate (e.g., the OverflowError caught in _n_to_index_) to avoid lengthy stack traces like

...
/local/cheuberg/local/sage-9.3.rc3/local/lib/python3.6/site-packages/sage/combinat/words/word_char.pyx in sage.combinat.words.word_char.WordDatatype_char._set_data (build/cythonized/sage/combinat/words/word_char.c:3830)()
    110         for i in range(self._length):
--> 111             self._data[i] = data[i]
    112 

OverflowError: can't convert negative value to unsigned char

During handling of the above exception, another exception occurred:

ValueError                                Traceback (most recent call last)
<ipython-input-1-b7387a9417cd> in <module>
      2 Seq2._n_to_index_(Integer(6))
      3 word: Integer(11)
----> 4 Seq2._n_to_index_(-Integer(1))

/local/cheuberg/local/sage-9.3.rc3/local/lib/python3.6/site-packages/sage/combinat/k_regular_sequence.py in _n_to_index_(self, n)
    353             return W(n.digits(self.k))
    354         except OverflowError:
--> 355             raise ValueError('value {} of index is negative'.format(n))

ValueError: value -1 of index is negative

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2021

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

89f656bTrac #21295 review issue 33: rename to number_of_zeros (as it should be)
2ddb686Trac #21295 review issue 7: document accessing coefficients
13ad4a1Trac #21295 review issue 29: notice minimize vs field
42606e3Merge tag '9.3' into t/21295/sequences/recognizable
1dbecfdMerge branch 't/21295/sequences/recognizable' into t/21203/sequences/k-regular

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2021

Changed commit from c4bf7ae to 1dbecfd

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 11, 2021

Changed commit from 1dbecfd to d532b56

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 11, 2021

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

d532b56Trac #21203 review issue 10: use "raise ... from None" where approriate

@dkrenn
Copy link
Contributor Author

dkrenn commented May 11, 2021

comment:41

Replying to @cheuberg:

Thank you for your changes.
9. What was the motivation to remove "If not specified (None), then the integer ring is used." ?

I believe that there is no obvious/canonical base ring for regular sequences, so I think, there should not be set a somehow artificial choice. (BTW, never used in the the tests, except for one specific test on this default argument.)

  1. I suggest to use raise ... from None where appropriate (e.g., the OverflowError caught in _n_to_index_) to avoid lengthy stack traces like [...]

Yes, indeed, this output is quite lengthy and does not bring anything. Done.

@cheuberg
Copy link
Contributor

comment:42

Thank you, all seems to be fine. For the record: the lint warnings do not concern the files changed in this ticket.

@vbraun
Copy link
Member

vbraun commented Jun 6, 2021

Changed branch from u/dkrenn/sequences/k-regular to d532b56

@vbraun vbraun closed this as completed in 9f3988d Jun 6, 2021
dkrenn added a commit to dkrenn/sage that referenced this issue May 25, 2023
…gular-guess

* t/21319/sequences/rec-hash: (11520 commits)
  Trac sagemath#21319: fixup due to changes in dependencies
  Trac sagemath#21318: fixup due to recent changes in dependencies
  Updated SageMath version to 9.3
  build/pkgs/fplll/spkg-install.in: Configure --without-qd if we use gcc from spkg
  build/pkgs/fplll/spkg-configure.m4: Add depcheck on gcc
  build/pkgs/ppl/spkg-configure.m4: Add depcheck on gcc
  build/pkgs/brial/spkg-configure.m4: Add depcheck on gcc
  build/pkgs/{freetype,libgd}/spkg-configure.m4: Add depcheck for gcc
  build/pkgs/zeromq/spkg-configure.m4: Add depcheck for gcc
  build/pkgs/ntl/spkg-configure.m4: Add depcheck for gcc
  Trac sagemath#21295 review issue 29: notice minimize vs field
  Trac sagemath#21295 review issue 7: document accessing coefficients
  Trac sagemath#21295 review issue 33: rename to number_of_zeros (as it should be)
  Trac sagemath#21203 review issue 4: rename to coefficient ring
  Trac sagemath#21295: rename to coefficient_ring
  Trac sagemath#21203 review issue 3: example for __getitem__ and __iter__
  Trac sagemath#21203 review issue 2: extend odds in Pascal's triangle
  Trac sagemath#21203 review issue 1: better binary sum of digits
  Trac sagemath#21203 review issue 5: meaningful error message in _n_to_index_
  Trac sagemath#21203 review issue 8: resolve "`n`th" in docstring
  ...
dkrenn added a commit to dkrenn/sage that referenced this issue May 25, 2023
…gular-guess

* t/21319/sequences/rec-hash:
  Trac sagemath#21319: fix punctuation
  Trac sagemath#21318: rmul/lmul preserve identity for multiplying by 1
  Trac sagemath#21325: fixup test .subsequence being identity
  Trac sagemath#21318: rmul/lmul preserve identity for multiplying by 1
  Trac sagemath#21325: use (new) decorator minimize_result
  Trac sagemath#21325: remove empty lines, fix punctuation
  Trac sagemath#21325: remove iteritems
  Trac sagemath#21318: fix rmul/lmul issues
  Trac sagemath#21318: use "correct" 1
  Trac sagemath#21318: use tensor_product in method hadamard_product
  Trac sagemath#21318: use is_trivial_zero in doctest of _neg_
  Trac sagemath#21318: use (new) .linear_representation in doctests
  Trac sagemath#21318: fix empty lines and punctuation
  Trac sagemath#21318: decorator minimize_result
  Trac sagemath#21203 review issue 10: use "raise ... from None" where approriate
dkrenn added a commit to dkrenn/sage that referenced this issue Jul 4, 2023
…ar-warning

* t/21204/sequences/k-regular-guess: (11522 commits)
  Trac sagemath#21204: fixup code and tests
  Trac sagemath#21204: cherry-pick to avoid merge conflict
  Trac sagemath#21319: fixup due to changes in dependencies
  Trac sagemath#21318: fixup due to recent changes in dependencies
  Updated SageMath version to 9.3
  build/pkgs/fplll/spkg-install.in: Configure --without-qd if we use gcc from spkg
  build/pkgs/fplll/spkg-configure.m4: Add depcheck on gcc
  build/pkgs/ppl/spkg-configure.m4: Add depcheck on gcc
  build/pkgs/brial/spkg-configure.m4: Add depcheck on gcc
  build/pkgs/{freetype,libgd}/spkg-configure.m4: Add depcheck for gcc
  build/pkgs/zeromq/spkg-configure.m4: Add depcheck for gcc
  build/pkgs/ntl/spkg-configure.m4: Add depcheck for gcc
  Trac sagemath#21295 review issue 29: notice minimize vs field
  Trac sagemath#21295 review issue 7: document accessing coefficients
  Trac sagemath#21295 review issue 33: rename to number_of_zeros (as it should be)
  Trac sagemath#21203 review issue 4: rename to coefficient ring
  Trac sagemath#21295: rename to coefficient_ring
  Trac sagemath#21203 review issue 3: example for __getitem__ and __iter__
  Trac sagemath#21203 review issue 2: extend odds in Pascal's triangle
  Trac sagemath#21203 review issue 1: better binary sum of digits
  ...
dkrenn added a commit to dkrenn/sage that referenced this issue Jul 4, 2023
…ar-warning

* t/21204/sequences/k-regular-guess:
  Trac sagemath#21204: fix punctuation
  Trac sagemath#21319: fix punctuation
  Trac sagemath#21318: rmul/lmul preserve identity for multiplying by 1
  Trac sagemath#21325: fixup test .subsequence being identity
  Trac sagemath#21318: rmul/lmul preserve identity for multiplying by 1
  Trac sagemath#21325: use (new) decorator minimize_result
  Trac sagemath#21325: remove empty lines, fix punctuation
  Trac sagemath#21325: remove iteritems
  Trac sagemath#21318: fix rmul/lmul issues
  Trac sagemath#21318: use "correct" 1
  Trac sagemath#21318: use tensor_product in method hadamard_product
  Trac sagemath#21318: use is_trivial_zero in doctest of _neg_
  Trac sagemath#21318: use (new) .linear_representation in doctests
  Trac sagemath#21318: fix empty lines and punctuation
  Trac sagemath#21318: decorator minimize_result
  Trac sagemath#21203 review issue 10: use "raise ... from None" where approriate
dkrenn added a commit to dkrenn/sage that referenced this issue Aug 3, 2023
* t/21343/k-regular-warning: (11523 commits)
  Trac sagemath#21343: adapt to removed transpose-property in dependency
  Trac sagemath#21204: fixup code and tests
  Trac sagemath#21204: cherry-pick to avoid merge conflict
  Trac sagemath#21319: fixup due to changes in dependencies
  Trac sagemath#21318: fixup due to recent changes in dependencies
  Updated SageMath version to 9.3
  build/pkgs/fplll/spkg-install.in: Configure --without-qd if we use gcc from spkg
  build/pkgs/fplll/spkg-configure.m4: Add depcheck on gcc
  build/pkgs/ppl/spkg-configure.m4: Add depcheck on gcc
  build/pkgs/brial/spkg-configure.m4: Add depcheck on gcc
  build/pkgs/{freetype,libgd}/spkg-configure.m4: Add depcheck for gcc
  build/pkgs/zeromq/spkg-configure.m4: Add depcheck for gcc
  build/pkgs/ntl/spkg-configure.m4: Add depcheck for gcc
  Trac sagemath#21295 review issue 29: notice minimize vs field
  Trac sagemath#21295 review issue 7: document accessing coefficients
  Trac sagemath#21295 review issue 33: rename to number_of_zeros (as it should be)
  Trac sagemath#21203 review issue 4: rename to coefficient ring
  Trac sagemath#21295: rename to coefficient_ring
  Trac sagemath#21203 review issue 3: example for __getitem__ and __iter__
  Trac sagemath#21203 review issue 2: extend odds in Pascal's triangle
  ...
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