tp_new leads to munmap_chunk(): invalid pointer segfault #1337
Comments
|
comment:1
Applying Robert's patch no-collect-interger.patch from http://sage.math.washington.edu/home/mabshoff/no-collect-integer.patch prevents the issue from occurring by incrementing the refcount if the dummy int. That way it is never reaped by the garbage collector. Cheers, Michael |
|
comment:2
Replying to @sagetrac-mabshoff:
Oops, I messed up somehow and the patch doesn't fix the problem. Investigating what went wrong ... Cheers, Michael |
|
Attachment: 1337.diff.gz |
|
comment:3
The new version of Cython handle tp_new in a more efficient manner, try applying the above patch. (In any case this should be applied to SAGE even if it doesn't fix the issue.) |
|
comment:4
Applying the patch and rebuilding all of Sage with the latest Cython (see #1801) does not solve the issue. Cheers, Michael |
|
comment:5
I merged 1337.diff into Sage 2.10.1.alpha1. Last time I checked it didn't fix the original problem, so stay tuned. Cheers, Michael |
|
comment:6
When I apply the patch I get double frees on sage.math: So I am reverting this for now. I also had to apply the no-collect-integer.patch to make the double frees go away again. This is very, very odd to say the least. Cheers, Michael |
|
comment:7
The presence, or absence, of this patch should not impact an error like this--are you sure it is due to this patch (and not some non-deterministic thing)? |
|
comment:8
I am attaching a patch that should fix the issue, as the elements of the structure used by the integers were in the wrong order, but I don't have a easy way to reproduce this, so testing would be appreciated. |
|
Fixed another tiny error. |
|
comment:9
Attachment: integers.pyx.patch.gz |
|
comment:10
The ZZ constructor breaks in the case of x=NULL because it was relying on broken code, patch coming up as soon as it make checks. |
|
Attachment: integer.pyx.patch.gz Fixed doctest issues |
|
comment:11
The last patch correctly solves all issues, and needs testing. (Ignore the first one). |
|
comment:12
It looks good to me, though I haven't ever been able to reproduce the error so someone else needs to test it. 1337.diff should be applied (though it doesn't fix the bug, but it simplifies things) integers.pyx.patch should NOT be applied (or returned integers might not have value 0) integer.pyx.patch should be applied. |
|
comment:13
Replying to @robertwb:
Can somebody please open one or two new tickets (depending if the patches do something differen), so I can merge them. Last time I tried this bug here wasn't fixed, so this way we can cleanly closed two issues. Cheers, Michael |
|
comment:14
Move integer.pyx.patch and structure fix to 2363. |
|
comment:15
Robert: Can you open another ticket for the 1337.diff patch, so that it can be reviewed. Or do you think it resolves this issue? Since Gary moved integer.pyx.patch to #2363 and you gave it a positive review I transferred the positive review there, merged it and then closed the ticket. Cheers, Michael |
|
Attachment: trac_1337.patch.gz double free fix (needs test all) |
|
Attachment: trac_1337-workaround.patch.gz This patch introduces a workaround - but that causes leaks if the integer pool is exhausted |
|
comment:17
Rubber stamp positive review |
vbraun
pushed a commit
that referenced
this issue
Apr 6, 2023
This adds an aperiodic monotile to the many examples in the documentation of the `polygon2d` function. https://arxiv.org/abs/2303.10798 <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [ ] I have linked an issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35331 Reported by: Moritz Firsching Reviewer(s): Matthias Köppe
vbraun
pushed a commit
that referenced
this issue
Apr 6, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description We make some optimizations to the computation of `squarefree_decomposition()` for polynomials over finite fields. This is a followup to #35323. We make improvements by only getting the parent of `T0` once and manually keep track of its degree. The current branch ```python sage: x = GF(4)["x"].gen() sage: p = x^2 + 1 sage: %timeit p.squarefree_decomposition() 140 µs ± 22.1 µs per loop (mean ± std. dev. of 7 runs, 10,000 loops each) ``` versus with #35323: ```python 168 µs ± 5.28 µs per loop (mean ± std. dev. of 7 runs, 10,000 loops each) ``` versus 10.0.beta5: ```python 158 µs ± 6.74 µs per loop (mean ± std. dev. of 7 runs, 10,000 loops each) ``` <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies - #35323 Based on this change and needs to avoid conflicts. URL: #35334 Reported by: Travis Scrimshaw Reviewer(s): Rémy Oudompheng
vbraun
pushed a commit
that referenced
this issue
Apr 6, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description ipython 8.11 throws new warnings "UserWarning: Shell was already running a gui event loop for sage; switching to sage." when running some tests. This is because sage indeed tries to install the same gui event loop for ipython again and again. The fix is checking if the ipython input hook is already set to the sage input hook, in that case we don't install the gui event loop again. In the test, we run `install()` twice so we can verify that running it multiple times won't cause a warning anymore. <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [x] I have created tests covering the changes. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> ### Replaces: #35235 Since gh doesn't allow to change the branch on a PR. URL: #35337 Reported by: Gonzalo Tornaría Reviewer(s): François Bissey
vbraun
pushed a commit
that referenced
this issue
Apr 6, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description Some develop packages (such as pytest) were missing from the `environment-dev.yml` file. <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [ ] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35349 Reported by: Tobias Diez Reviewer(s): Matthias Köppe
vbraun
pushed a commit
that referenced
this issue
Apr 6, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description For some python packages, conda installed an incompatible version which was then uninstalled and replaced by the correct version during the `pip install src` step. This PR adds the missing version specifiers in the conda dist files. It also adds a check in the conda ci workflow and runs the conda workflow automatically on package updates (using the label) to prevent such mismatches in the future. <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [ ] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35350 Reported by: Tobias Diez Reviewer(s): Matthias Köppe, Tobias Diez
vbraun
pushed a commit
that referenced
this issue
Apr 6, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description Since #35093, sage no longer builds with gap 4.11. With this PR the new minimum version is correctly specified for the conda installation. This currently doesn't work as there is no suitable conda package yet, see https://github.com/conda- forge/gap-feedstock/pull/68. However, better fail with a good error message then some random warning during build. It would be nice if in the future breaking package updates would only happen if conda provides a suitable package. <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35351 Reported by: Tobias Diez Reviewer(s): Dima Pasechnik
vbraun
pushed a commit
that referenced
this issue
Apr 6, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description Add instructions in the docs on how to update existing conda environment. <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [ ] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35352 Reported by: Tobias Diez Reviewer(s): Matthias Köppe
vbraun
pushed a commit
that referenced
this issue
Apr 6, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description vscode uses katex to render the latex output of jupyter cells. However, katex doesn't support mbox (see KaTeX/KaTeX#2050). Thus we replace `\mbox` with `\text`. <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [ ] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35353 Reported by: Tobias Diez Reviewer(s): Eric Gourgoulhon
vbraun
pushed a commit
that referenced
this issue
Apr 13, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description fix pycodestyle E502 warning in schemes/ and combinat/ <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [ ] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35318 Reported by: Frédéric Chapoton Reviewer(s): Frédéric Chapoton, Kwankyu Lee
vbraun
pushed a commit
that referenced
this issue
Apr 23, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description We make `sage.quadratic_forms` ready for modularized distributions by: - moving imports from some packages, such as `sage.rings.padics.factory`, into methods - using `lazy_import` to provide the methods of `QuadraticForm` implemented in separate modules - adding `# optional` annotations to doctests Also some improvements to the code style of doctests and the Sphinx markup of some docstrings have been made. This has been and can be tested by merging into: - #35095 **sagemath-polyhedra** ships vector spaces etc. and now also portions of `sage.quadratic_forms` – see #35095 /files#diff- f4c7c1beea87ae44c426de0a55459daef63b75b701b72bc136d88f4e7c9285cfR104 Resolves #35243 Part of: - #29705 <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> - Depends on #35248 - Depends on #35166 URL: #35305 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe
vbraun
pushed a commit
that referenced
this issue
Apr 23, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description We move the `...MatrixGroup_gap` classes to separate modules named `..._gap`. This is part of: - #29705 <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> - Depends on #35099 - Depends on #35119 URL: #35306 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe
vbraun
pushed a commit
that referenced
this issue
May 22, 2023
…migration
…to GitHub (no more sagetrac-mirror)
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes #1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description
We update the documentation on portability testing.
- Instead of referring to trac and sagetrac-mirror, we direct users to
use their personal fork
- We remove the description of a method using pull requests because
#34942 disabled it
- We update according to changed defaults on GH Actions.
<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes #1337" -->
### 📝 Checklist
<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [x] I have linked an issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.
### ⌛ Dependencies
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
Test running at:
https://github.com/mkoeppe/sage/actions/runs/4911075567/jobs/8768884045
(the "standard", "minimal" etc. jobs without "-pre" should now work).
URL: #35108
Reported by: Matthias Köppe
Reviewer(s): Dima Pasechnik, Matthias Köppe, Tobias Diez
vbraun
pushed a commit
that referenced
this issue
May 22, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> Previous fixes for tox 4.x were not complete, as observed in #35095 (comment) The files `pkgs/*/tox.ini` now requires tox >= 4.2. tox 3.x and 4.x auto-provision a suitable version, so it is not immediately necessary to upgrade our `tox` package (or tighten the version requirements in `build/pkgs/tox/spkg-configure.m4`. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35208 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee
vbraun
pushed a commit
that referenced
this issue
May 22, 2023
…odule
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes #1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description
The goal of this PR is to implement the logarithm and the exponential of
a Drinfeld module.
### Background material
Let $\phi:T \mapsto \gamma(T) + g_1 \tau + \cdots + g_r \tau^r$ with
$g_r\neq 0$ be a rank $r$ Drinfeld module and let $\Lambda\subset
\mathbb{C}\_{\infty}$ be the associated rank $r$ lattice. Then recall
that the exponential $e\_{\phi}:\mathbb{C}\_{\infty} \rightarrow
\mathbb{C}\_{\infty}$ of the Drinfeld module $\phi$ is defined by:
$$
e_{\phi}(z) := z\prod_{\lambda\in \Lambda\setminus \{0\}} \left( 1 -
z/\lambda \right).
$$
It can be viewed as a power series in $z$:
$$
e_{\phi}(z) = z + \alpha_1 z^{q} + \alpha_2 z^{q^2} + \alpha_3 z^{q^3} +
\cdots
$$
Moreover, it satisfies the following functional equation:
$$
e_{\phi}(az) = \phi_a(e_{\phi}(z)).
$$
The logarithm of $\phi$ is defined to be the compositional inverse of
$e_{\phi}$, denoted $\mathrm{log}\_{\phi}$. By applying
$\mathrm{log}\_{\phi}$ on both side of the functional equation, we
obtain the following recurrence relation:
$$
\beta_k = \frac{1}{\gamma(T) - \gamma(T)^{q^i - 1}} \sum_{i = 0}^{k-1}
\beta_i g_{k-i}^{q^i}
$$
where $\beta_k$ is the $q^k$-th coefficient of $\mathrm{log}_{\phi}$ and
$\beta_0 = 1$.
### Examples
```
sage: A = GF(2)['T']
sage: K.<T> = Frac(A)
sage: phi = DrinfeldModule(A, [T, 1])
sage: q = A.base_ring().cardinality()
sage: log = phi.logarithm(); log
z + ((1/(T^2+T))*z^2) + ((1/(T^6+T^5+T^3+T^2))*z^4) + O(z^7)
sage: log[q] == -1/((T**q - T))
True
sage: log[q**2] == 1/((T**q - T)*(T**(q**2) - T))
True
sage: log[q**3] == -1/((T**q - T)*(T**(q**2) - T)*(T**(q**3) - T))
True
```
### Ideas
The idea here is to first compute the logarithm of a Drinfeld module
using lazy power series and then compute its compositional inverse to
find its exponential.
<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes #1337" -->
### 📝 Checklist
<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [ ] I have linked an issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation accordingly.
### ⌛ Dependencies
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
Depends on #35026 because this PR implements Drinfeld modules.
CC: @xcaruso @ymusleh @kryzar
URL: #35260
Reported by: David Ayotte
Reviewer(s): Antoine Leudière, David Ayotte, Xavier Caruso
vbraun
pushed a commit
that referenced
this issue
Jun 3, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> This PR is a continuation of https://github.com/sagemath/trac-to- github/pull/187. It implements GitHub actions to synchronize the labels migrated from Trac selection lists (according to issue sagemath/trac-to-github#117). These are the priority and state labels (type labels are not considered here since their meaning has changed). The bot will not be active immediatly after this PR is merged. It needs to be activated by a maintainer of the repository by adding the variable `SYNC_LABELS_ACTIVE` with value `yes` under ``` Settings -> Secrets and variables -> Actions ``` At the moment the script takes care that there is just one item present from each list and it sets `s: needs review` on a non draft PR opening. Furthermore, it reacts on review approval and change requests setting according labels automatically (after some checks). Conversely, setting a state label will lead to automatic approval resp. change request under certain circumstances. If there is need to reject it a warning comment is posted. For a detailed description of the workflows see the flowcharts below. In summary, the bot keeps the meaning consistent along the rows in the table below: | Trac status | GH label | closed | draft | review decision | | - | - | - | - | - | | new | | No | Yes | None | | needs_info | s: needs info | No | No / Yes | Any | | needs review | s: needs review | No | No | Any | | needs work | s: needs work | No | No | CHANGES REQUESTED | | positive review | s: positive review | No | No | APPROVED | | closed | | Yes | No / Yes | Any | The review decision is not given explicitly on all cases. Implicitly it can be read of the list of all reviews. So if there is at least one review requesting changes younger than any commit the review decision is interpreted as `CHANGES REQUESTED`. If there is at least one approved review younger than any commit but none requesting changes the review decision is interpreted as `APPROVED`. Please consider this implementation just as a base of discussion. #### Open problems and questions * At the moment there is no synchronization between issue labels of selection lists and corresponding labels of associated PRs * Does it make sense to have state labels on issues? In the current version of the PR I reject all but `s: needs info`. * In general the relation between issues and PRs can be n:m. Shall we accept different priorities among them or shall we put all these to the maximum. I think at least the priority of a PR should be the maximum of all issues fixed by it. #### Notes on testing The failure of the *Synchronize selection list labeled / synchronize (pull_request)* check is to be expected (see #35172 (comment)) since it is trying to download the python file from the upstream repository (`sagemath/sage`). Therefore testing the script must be done relative to my fork (`soehms/sage`). First examples are soehms#1 and soehms#2. It would be helpful if others would open PRs there in order to test the review functionality which I can't do by myself. ### Flowcharts #### What happens when a PR changes its state? ##### What happens when a PR is opened? ```mermaid --- title: open a PR --- flowchart LR %% vertices trigger([open\n]) add_needs_review(['s: needs review'\n label added]) nothing([do nothing]) is_draft{is PR\n a draft?} %% edges trigger ---> is_draft is_draft -- true ---> nothing is_draft -- false ---> add_needs_review ``` ##### What happens when a PR is closed or reopened or converted to a draft? ```mermaid --- title: close, reopen or convert a PR to a draft --- flowchart LR %% vertices trigger([close, reopen or\n convert to draft]) remove_all_labels([remove all\n state labels]) %% edges trigger ---> remove_all_labels ``` ##### What happens when a draft is ready for review or the branch of a PR is synchronized? ```mermaid --- title: draft ready for review or branch synchronized --- flowchart LR %% vertices trigger([ready for\n review]) nothing([do nothing]) add_needs_review(['s: needs review'\n label selected]) needs_review_valid[[needs review\n valid?]] %% edges trigger ---> needs_review_valid needs_review_valid -- true ---> add_needs_review needs_review_valid -- false ---> nothing ``` ```mermaid --- title: needs review valid? --- flowchart LR %% vertices needs_review_valid([needs review\n valid?]) true([true]) false([false]) needs_work_valid[[needs work\n valid?]] positive_review_valid[[positive review\n valid?]] is_draft{is PR\n a draft?} %% edges needs_review_valid ---> is_draft is_draft -- yes ---> false is_draft -- no ---> needs_work_valid needs_work_valid -- true ---> false needs_work_valid -- false ---> positive_review_valid positive_review_valid -- true ---> false positive_review_valid -- false ---> true ``` ```mermaid --- title: needs work valid? --- flowchart LR %% vertices needs_work_valid([needs work\n valid?]) true([true]) false([false]) proper_review_exists{proper review\n after most recent\n commit exists?} review_decision_exists{review\n decision\n exists?} review_decision_request_changes{review\n decision is\n CHANGES\n REQUESTED?} any_review_request_changes{any proper\n review requests\n changes?} %% edges needs_work_valid --> proper_review_exists proper_review_exists -- yes ---> review_decision_exists proper_review_exists -- no ---> false review_decision_exists -- yes ---> review_decision_request_changes review_decision_exists -- no ---> any_review_request_changes review_decision_request_changes -- yes --> true review_decision_request_changes -- no --> false any_review_request_changes -- yes ---> true any_review_request_changes -- no ---> false ``` Here, proper means that the review is more than a comment. ```mermaid --- title: positive review valid? --- flowchart LR %% vertices positive_review_valid([positive review\n valid?]) true([true]) false([false]) proper_review_exists{proper review\n after most recent\n commit exists?} review_decision_exists{review\n decision\n exists?} review_decision_approved{review\n decision is\n APPROVED?} all_proper_reviews_approved{all proper\n reviews\n approved?} %% edges positive_review_valid --> proper_review_exists proper_review_exists -- yes ---> review_decision_exists proper_review_exists -- no ---> false review_decision_exists -- yes ---> review_decision_approved review_decision_exists -- no ---> all_proper_reviews_approved review_decision_approved -- yes --> true review_decision_approved -- no --> false all_proper_reviews_approved -- yes ---> true all_proper_reviews_approved -- no ---> false ``` Here, proper means that the review is more than a comment. ##### What happens when changes are requested for a PR? ```mermaid --- title: request changes --- flowchart LR %% vertices trigger([request\n changes\n]) add_needs_work(['s: needs work'\n label selected]) nothing([do nothing]) needs_work_valid[[needs work\n valid?]] %% edges trigger ---> needs_work_valid needs_work_valid -- true ---> add_needs_work needs_work_valid -- false ---> nothing ``` ##### What happens when a PR is approved? ```mermaid --- title: approve --- flowchart LR %% vertices trigger([approve\n]) select_positive_review(['s: positive review'\n label selected]) nothing([do nothing]) positive_review_valid[[positive review\n valid?]] actor_authorized{is actor\n a member of\n Triage?} %% edges trigger ---> actor_authorized actor_authorized -- yes ---> positive_review_valid actor_authorized -- no ---> nothing positive_review_valid -- true ---> select_positive_review positive_review_valid -- false ---> nothing ``` #### What happens when state or priority labels are changed? ##### What happens when adding `s: positive review` to a PR? ```mermaid --- title: add the positive review label --- flowchart LR %% vertices trigger([label\n 's: positive review'\n added]) positive_review_valid[[positive review\n valid?]] approve_pr([approve]) remove_other_labels([remove other\n state labels]) approve_allowed[[approve\n allowed?]] reject_label_addition[[reject\n label\n addition]] %% edges trigger --> positive_review_valid positive_review_valid -- yes ---> remove_other_labels positive_review_valid -- no ---> approve_allowed approve_allowed -- yes ---> approve_pr approve_allowed -- no ---> reject_label_addition approve_pr --> remove_other_labels ``` ```mermaid --- title: approve allowed? --- flowchart LR %% vertices trigger([approve\n allowed?]) true([true]) false([false]) review_of_member_exists{review\n of a member\n exists?} review_of_others_request_changes{changes\n requested by\n someone\n else exists?} actor_valid[[actor valid?]] %% edges trigger --> review_of_member_exists review_of_member_exists -- yes ---> review_of_others_request_changes review_of_member_exists -- no ---> false review_of_others_request_changes -- yes ---> false review_of_others_request_changes -- no ---> actor_valid actor_valid -- yes ---> true actor_valid -- no ---> false ``` Here, only reviews of someone else are considered which are more recent than any commit. ```mermaid --- title: actor valid --- flowchart LR %% vertices actor_valid([actor valid?]) true([true]) false([false]) actor_is_author{actor is\n author?} other_reviews{reviews of\n someone else\n exists?} other_commits{commits of\n someone else\n exists?} %% edges actor_valid --> actor_is_author actor_is_author -- yes ---> other_reviews actor_is_author -- no ---> true other_reviews -- yes ---> other_commits other_reviews -- no ---> false other_commits -- yes ---> true other_commits -- no ---> false ``` ```mermaid --- title: reject label addition --- flowchart LR %% vertices reject_label_addition([reject\n label\n addition]) add_warning_comment([add\n warning\n comment]) remove_label([remove label]) %% edges reject_label_addition --> add_warning_comment add_warning_comment --> remove_label ``` ##### What happens when adding `s: needs work` to a PR? ```mermaid --- title: add the needs work label --- flowchart LR %% vertices trigger([label\n 's: needs work'\n added]) request_changes([request changes]) remove_other_labels([remove other\n state labels]) reject_label_addition[[reject\n label\n addition]] needs_work_valid[[needs work\n valid?]] is_draft{is PR\n a draft?} %% edges trigger --> needs_work_valid needs_work_valid -- true ---> remove_other_labels needs_work_valid -- false ---> is_draft is_draft -- yes ---> reject_label_addition is_draft -- no ---> request_changes request_changes --> remove_other_labels ``` ##### What happens when adding `s: needs review` to a PR? ```mermaid --- title: add the needs review label --- flowchart LR %% vertices trigger([label\n 's: needs review'\n added]) mark_as_ready([mark as\n ready for\n review]) remove_other_labels([remove other\n state labels]) reject_label_addition[[reject\n label\n addition]] needs_review_valid[[needs review\n valid?]] is_draft{is PR\n a draft?} %% edges trigger --> needs_review_valid needs_review_valid -- true ---> remove_other_labels needs_review_valid -- false ---> is_draft is_draft -- yes ---> mark_as_ready is_draft -- no ---> reject_label_addition mark_as_ready --> remove_other_labels ``` ##### What happens when adding `s: needs info`? ```mermaid --- title: add the needs info label --- flowchart LR %% vertices trigger([label added]) remove_other_labels([remove other\n state labels]) %% edges trigger --> remove_other_labels ``` ##### What happens when adding a state label to an issue? ```mermaid --- title: add a state label to an issue --- flowchart LR %% vertices trigger([label added]) reject_label_addition([reject\n label\n addition]) nothing([do nothing]) is_needs_info{is label\n 's: needs info'?} %% edges trigger --> is_needs_info is_needs_info -- yes ---> nothing is_needs_info -- no ---> reject_label_addition ``` ##### What happens when removing a state label from a PR? ```mermaid --- title: remove a state label --- flowchart LR %% vertices trigger([label removed]) nothing([do nothing]) reject_label_removal([reject\n label\n removal]) is_needs_info{is label\n 's: needs info'?} %% edges trigger --> is_needs_info is_needs_info -- yes ---> nothing is_needs_info -- no ---> reject_label_removal ``` Nothing happens when a state label is removed from an issue. ##### What happens when adding a priority label? ```mermaid --- title: add a priority label --- flowchart LR %% vertices trigger([label added]) remove_other_labels([remove other\n priority labels]) %% edges trigger --> remove_other_labels ``` ##### What happens when removing a priority label? ```mermaid --- title: remove a priority label --- flowchart LR %% vertices trigger([label removed]) reject_label_removal([reject\n label\n removal]) %% edges trigger --> reject_label_removal ``` ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35172 Reported by: Sebastian Oehms Reviewer(s): Kwankyu Lee, Sebastian Oehms, Tobias Diez
vbraun
pushed a commit
that referenced
this issue
Jul 9, 2023
...so that relevant information appears when hovering over items of the issue list, and so that one can read the actual issue on the issue page without scrolling over a full page of boilerplate. Also remove the title from the Description section of the PR template (same rationale). Any opinions on that? <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [ ] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35295 Reported by: Marc Mezzarobba Reviewer(s): Dima Pasechnik, Marc Mezzarobba, Tobias Diez
vbraun
pushed a commit
that referenced
this issue
Jul 30, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description The goal of this PR is to implement basic j-invariants of Drinfeld module as defined by Potemine in Potemine, I.Y. Minimal Terminal ℚ-Factorial Models of Drinfeld Coarse Moduli Schemes. Mathematical Physics, Analysis and Geometry 1, 171–191 (1998). https://doi.org/10.1023/A:1009724323513 These j-invariants allows to determine whether two Drinfeld `Fq[T]`-modules of arbitrary ranks are isomorphic or not. @kryzar @ymusleh @xcaruso <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [] I have linked an issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> Depends on #35026 because this PR implements Drinfeld modules. URL: #35057 Reported by: David Ayotte Reviewer(s): Antoine Leudière, David Ayotte, Xavier Caruso
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Aug 23, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description <!-- Describe your changes here in detail --> Update to latest in 0.x series https://cython.readthedocs.io/en/latest/src/changes.html#id35 <!-- Why is this change required? What problem does it solve? --> Preparation for Python 3.12 <!-- If it resolves an open issue, please link to the issue here. For example "Closes sagemath#1337" --> Fixes sagemath#34897 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: sagemath#35084 Reported by: Matthias Köppe Reviewer(s): Dima Pasechnik
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Aug 27, 2023
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description <!-- Describe your changes here in detail --> Update to latest in 0.x series https://cython.readthedocs.io/en/latest/src/changes.html#id35 <!-- Why is this change required? What problem does it solve? --> Preparation for Python 3.12 <!-- If it resolves an open issue, please link to the issue here. For example "Closes sagemath#1337" --> Fixes sagemath#34897 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: sagemath#35084 Reported by: Matthias Köppe Reviewer(s): Dima Pasechnik
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Oct 1, 2023
### 📚 Description Two days ago [github announced the public beta of merge queues](https://github.blog/changelog/2023-02-08-pull-request-merge- queue-public-beta/). The idea is pretty similar the current release/merge process (as far as I understand it): Given you want to merge three PRs into `develop`, you add them to the merge queue and github will create three temporary branches: - PR1 - PR1 + PR2 merged - PR1 + PR2 + PR3 merged It then runs all checks with the `merge_group` trigger on each branch. Once all checks for all these temporary branches are successful, the whole group is merged into `develop`. If there are merge conflicts, say between PR3 and PR1, or the build for the PR1+2+3 branch fails, then PR3 is removed from the merge queue and only the first two PRs are merged into `develop`. In this way, one could never end up in a situation where the checks are not passing in the develop branch, without requiring devs to always merge the latest develop branch themselves. (E.g., we will no longer hit the situation that PR2 introduces new linter rules which are not fullfiled by PR3 but both PRs are merged into develop). References: - https://docs.github.com/en/repositories/configuring-branches-and- merges-in-your-repository/configuring-pull-request-merges/managing-a- merge-queue - https://docs.github.com/en/actions/using-workflows/events-that- trigger-workflows#merge_group --- In this PR we add the merge_group trigger on the current PR-workflows, which enables to run these in the merge queue as well. --- Since this effectively implements the current version of "Volker's temporary private develop branch", I was wondering if this would be eneough to give more people rights to merge PRs. What else would be necessary for such a step? @vbraun <!-- Describe your changes in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes sagemath#1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [ ] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: sagemath#35062 Reported by: Tobias Diez Reviewer(s): Dima Pasechnik, Matthias Köppe, Tobias Diez
vbraun
pushed a commit
that referenced
this issue
Oct 8, 2023
### 📚 Description Two days ago [github announced the public beta of merge queues](https://github.blog/changelog/2023-02-08-pull-request-merge- queue-public-beta/). The idea is pretty similar the current release/merge process (as far as I understand it): Given you want to merge three PRs into `develop`, you add them to the merge queue and github will create three temporary branches: - PR1 - PR1 + PR2 merged - PR1 + PR2 + PR3 merged It then runs all checks with the `merge_group` trigger on each branch. Once all checks for all these temporary branches are successful, the whole group is merged into `develop`. If there are merge conflicts, say between PR3 and PR1, or the build for the PR1+2+3 branch fails, then PR3 is removed from the merge queue and only the first two PRs are merged into `develop`. In this way, one could never end up in a situation where the checks are not passing in the develop branch, without requiring devs to always merge the latest develop branch themselves. (E.g., we will no longer hit the situation that PR2 introduces new linter rules which are not fullfiled by PR3 but both PRs are merged into develop). References: - https://docs.github.com/en/repositories/configuring-branches-and- merges-in-your-repository/configuring-pull-request-merges/managing-a- merge-queue - https://docs.github.com/en/actions/using-workflows/events-that- trigger-workflows#merge_group --- In this PR we add the merge_group trigger on the current PR-workflows, which enables to run these in the merge queue as well. --- Since this effectively implements the current version of "Volker's temporary private develop branch", I was wondering if this would be eneough to give more people rights to merge PRs. What else would be necessary for such a step? @vbraun <!-- Describe your changes in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [ ] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35062 Reported by: Tobias Diez Reviewer(s): Dima Pasechnik, Matthias Köppe, Tobias Diez
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Nov 1, 2023
### 📚 Description Added 3 functions that deal with k-th roots of permutations: 1) Permutation.kth_roots(k) compute a iterator of over all k-th roots of self. 2) Permutation.has_kth_root(k) determines if self has a k-th root (or several). 3) Permutation.number_of_kth_roots(k) returns the number of k-th roots of self. Added integer_partition_with_given_parts(n, parts) in partition.py (for use in k-th roots computations): it creates a iterator over all partitions of n which parts are in the variable parts. <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes sagemath#1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [ ] I have linked an issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: sagemath#35053 Reported by: GermainPoullot Reviewer(s): Frédéric Chapoton, GermainPoullot, Martin Rubey, Travis Scrimshaw, Vincent Delecroix
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Nov 2, 2023
### 📚 Description Added 3 functions that deal with k-th roots of permutations: 1) Permutation.kth_roots(k) compute a iterator of over all k-th roots of self. 2) Permutation.has_kth_root(k) determines if self has a k-th root (or several). 3) Permutation.number_of_kth_roots(k) returns the number of k-th roots of self. Added integer_partition_with_given_parts(n, parts) in partition.py (for use in k-th roots computations): it creates a iterator over all partitions of n which parts are in the variable parts. <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes sagemath#1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [ ] I have linked an issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: sagemath#35053 Reported by: GermainPoullot Reviewer(s): Frédéric Chapoton, GermainPoullot, Martin Rubey, Travis Scrimshaw, Vincent Delecroix
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Nov 5, 2023
### 📚 Description Added 3 functions that deal with k-th roots of permutations: 1) Permutation.kth_roots(k) compute a iterator of over all k-th roots of self. 2) Permutation.has_kth_root(k) determines if self has a k-th root (or several). 3) Permutation.number_of_kth_roots(k) returns the number of k-th roots of self. Added integer_partition_with_given_parts(n, parts) in partition.py (for use in k-th roots computations): it creates a iterator over all partitions of n which parts are in the variable parts. <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes sagemath#1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [ ] I have linked an issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: sagemath#35053 Reported by: GermainPoullot Reviewer(s): Frédéric Chapoton, GermainPoullot, Martin Rubey, Travis Scrimshaw, Vincent Delecroix
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Dec 17, 2023
… to a subclass of `FiniteFamily`
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description
Fixes sagemath#31750
<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes sagemath#1337" -->
### 📝 Checklist
<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [ ] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [ ] I have linked an issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.
### ⌛ Dependencies
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
URL: sagemath#35121
Reported by: Matthias Köppe
Reviewer(s): Travis Scrimshaw
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Feb 22, 2024
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description Fixes sagemath#34890 Authors: @mkoeppe, @mantepse <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes sagemath#1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [ ] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [ ] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: sagemath#35103 Reported by: Matthias Köppe Reviewer(s): Martin Rubey, Matthias Köppe, Travis Scrimshaw
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Mar 24, 2024
… forms
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description
<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes sagemath#1337" -->
This PR implements the method `QuasiModularForms.basis_of_weight(k)`. It
returns a list of quasimodular forms which generate the space of weight
k. Some examples:
```
sage: QM = QuasiModularForms(1)
sage: QM.basis_of_weight(12)
[q - 24*q^2 + 252*q^3 - 1472*q^4 + 4830*q^5 + O(q^6),
1 + 65520/691*q + 134250480/691*q^2 + 11606736960/691*q^3 +
274945048560/691*q^4 + 3199218815520/691*q^5 + O(q^6),
1 - 288*q - 129168*q^2 - 1927296*q^3 + 65152656*q^4 + 1535768640*q^5 +
O(q^6),
1 + 432*q + 39312*q^2 - 1711296*q^3 - 14159664*q^4 + 317412000*q^5 +
O(q^6),
1 - 576*q + 21168*q^2 + 308736*q^3 - 15034608*q^4 - 39208320*q^5 +
O(q^6),
1 + 144*q - 17712*q^2 + 524736*q^3 - 2279088*q^4 - 79760160*q^5 +
O(q^6),
1 - 144*q + 8208*q^2 - 225216*q^3 + 2634192*q^4 + 1488672*q^5 + O(q^6)]
sage: QM = QuasiModularForms(Gamma1(3))
sage: QM.basis_of_weight(3)
[1 + 54*q^2 + 72*q^3 + 432*q^5 + O(q^6),
q + 3*q^2 + 9*q^3 + 13*q^4 + 24*q^5 + O(q^6)]
sage: QM.basis_of_weight(5)
[1 - 90*q^2 - 240*q^3 - 3744*q^5 + O(q^6),
q + 15*q^2 + 81*q^3 + 241*q^4 + 624*q^5 + O(q^6),
1 - 24*q - 18*q^2 - 1320*q^3 - 5784*q^4 - 10080*q^5 + O(q^6),
q - 21*q^2 - 135*q^3 - 515*q^4 - 1392*q^5 + O(q^6)]
```
CC: @videlec
### 📝 Checklist
<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [ ] I have linked an issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation accordingly.
### ⌛ Dependencies
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
URL: sagemath#35029
Reported by: David Ayotte
Reviewer(s): David Ayotte, grhkm21
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Sep 16, 2024
…base
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description
Building the large Cremona database fails like this on an x86_64 Fedora
Rawhide machine. (I captured the output from an old sagemath 9.6 build,
but the same error occurs with sagemath 9.7.)
```
Inserting allgens.90000-99999
Committing...
Traceback (most recent call last):
File "/builddir/build/BUILD/sage-9.6/cremona.sage.py", line 10, in
<module>
c._init_from_ftpdata('ecdata-2019-10-29')
File "/builddir/build/BUILDROOT/sagemath-9.6-
4.fc38.x86_64/usr/lib64/python3.11/site-
packages/sage/databases/cremona.py", line 1397, in _init_from_ftpdata
self.vacuum()
File "/builddir/build/BUILDROOT/sagemath-9.6-
4.fc38.x86_64/usr/lib64/python3.11/site-
packages/sage/databases/sql_db.py", line 2180, in vacuum
self.__connection__.execute('VACUUM')
sqlite3.OperationalError: cannot VACUUM from within a transaction
```
The issue is a missing database commit, which this PR adds.
<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes sagemath#1337" -->
### 📝 Checklist
<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [ ] I have linked an issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.
### ⌛ Dependencies
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
URL: sagemath#35050
Reported by: Jerry James
Reviewer(s): Vincent Macri
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Sep 19, 2024
…base
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description
Building the large Cremona database fails like this on an x86_64 Fedora
Rawhide machine. (I captured the output from an old sagemath 9.6 build,
but the same error occurs with sagemath 9.7.)
```
Inserting allgens.90000-99999
Committing...
Traceback (most recent call last):
File "/builddir/build/BUILD/sage-9.6/cremona.sage.py", line 10, in
<module>
c._init_from_ftpdata('ecdata-2019-10-29')
File "/builddir/build/BUILDROOT/sagemath-9.6-
4.fc38.x86_64/usr/lib64/python3.11/site-
packages/sage/databases/cremona.py", line 1397, in _init_from_ftpdata
self.vacuum()
File "/builddir/build/BUILDROOT/sagemath-9.6-
4.fc38.x86_64/usr/lib64/python3.11/site-
packages/sage/databases/sql_db.py", line 2180, in vacuum
self.__connection__.execute('VACUUM')
sqlite3.OperationalError: cannot VACUUM from within a transaction
```
The issue is a missing database commit, which this PR adds.
<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes sagemath#1337" -->
### 📝 Checklist
<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [ ] I have linked an issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.
### ⌛ Dependencies
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
URL: sagemath#35050
Reported by: Jerry James
Reviewer(s): Vincent Macri
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Sep 22, 2024
…base
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description
Building the large Cremona database fails like this on an x86_64 Fedora
Rawhide machine. (I captured the output from an old sagemath 9.6 build,
but the same error occurs with sagemath 9.7.)
```
Inserting allgens.90000-99999
Committing...
Traceback (most recent call last):
File "/builddir/build/BUILD/sage-9.6/cremona.sage.py", line 10, in
<module>
c._init_from_ftpdata('ecdata-2019-10-29')
File "/builddir/build/BUILDROOT/sagemath-9.6-
4.fc38.x86_64/usr/lib64/python3.11/site-
packages/sage/databases/cremona.py", line 1397, in _init_from_ftpdata
self.vacuum()
File "/builddir/build/BUILDROOT/sagemath-9.6-
4.fc38.x86_64/usr/lib64/python3.11/site-
packages/sage/databases/sql_db.py", line 2180, in vacuum
self.__connection__.execute('VACUUM')
sqlite3.OperationalError: cannot VACUUM from within a transaction
```
The issue is a missing database commit, which this PR adds.
<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes sagemath#1337" -->
### 📝 Checklist
<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
- [x] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [ ] I have linked an issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.
### ⌛ Dependencies
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
URL: sagemath#35050
Reported by: Jerry James
Reviewer(s): Vincent Macri
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When compiling python
--without-pymallocvalgrind detects the following problem:For the Linux PPC port this leads to the following crash on exit:
See also http://groups.google.com/group/sage-devel/t/e254485d2e367b20
This bug did not show in 2.8.7 on the same PPC hardware under Linux.
Component: memleak
Issue created by migration from https://trac.sagemath.org/ticket/1337
The text was updated successfully, but these errors were encountered: