Opened 8 years ago
Closed 3 years ago
#16564 closed enhancement (fixed)
Derivations for inseparable function fields
Reported by: | saraedum | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-8.7 |
Component: | commutative algebra | Keywords: | sd59, sd86.5 |
Cc: | Merged in: | ||
Authors: | Julian Rüth, Jeroen Sijsling | Reviewers: | Jeroen Sijsling, Julian Rüth, Stefan Wewers |
Report Upstream: | N/A | Work issues: | |
Branch: | f63c4ff (Commits, GitHub, GitLab) | Commit: | f63c4fff210aef4bbec4d9cce99bd2dc46ab0fbd |
Dependencies: | Stopgaps: |
Description
This ticket implements derivations for inseparable extensions of function fields.
Attachments (1)
Change History (45)
comment:1 Changed 8 years ago by
- Keywords sd59 added
comment:2 Changed 8 years ago by
- Branch set to u/saraedum/ticket/16564
- Created changed from 06/27/14 05:21:18 to 06/27/14 05:21:18
- Modified changed from 06/27/14 05:25:48 to 06/27/14 05:25:48
comment:3 Changed 8 years ago by
- Status changed from new to needs_review
comment:4 Changed 8 years ago by
- Commit set to 1e32daed9b798cecf0fe399b072f4ec55fdd2e26
comment:5 Changed 8 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:6 Changed 7 years ago by
- Status changed from needs_review to needs_work
- Work issues set to BuildFailed
comment:7 Changed 7 years ago by
- Commit changed from 1e32daed9b798cecf0fe399b072f4ec55fdd2e26 to cb3a16b73ddefcc853a17945efd9311baeb5acac
Branch pushed to git repo; I updated commit sha1. New commits:
724b02c | Merge branch 'develop' into t/16561/ticket/16561
|
ac8427a | Fixed format errors in docstrings
|
8406f2e | Merge branch 'u/saraedum/ticket/16561' of git://trac.sagemath.org/sage into t/16564/ticket/16564
|
716ec10 | Merge branch 'develop' into t/16562/ticket/16562
|
a2ab48b | Fixed docstring syntax error
|
cb3a16b | Merge branch 'u/saraedum/ticket/16562' of git://trac.sagemath.org/sage into t/16564/ticket/16564
|
comment:8 Changed 7 years ago by
- Status changed from needs_work to needs_review
comment:9 Changed 7 years ago by
- Commit changed from cb3a16b73ddefcc853a17945efd9311baeb5acac to 256f7cf8d519018cb1e0d204099f634cbf18ef7c
Branch pushed to git repo; I updated commit sha1. New commits:
256f7cf | Fixed doctests which failed due to changed printing of function fields and their morphisms
|
comment:10 Changed 6 years ago by
- Status changed from needs_review to needs_work
comment:11 Changed 5 years ago by
- Keywords sd86.5 added
comment:12 Changed 5 years ago by
- Work issues changed from BuildFailed to BuildFailed, merge conflict
comment:13 Changed 5 years ago by
comment:14 Changed 5 years ago by
- Commit changed from 256f7cf8d519018cb1e0d204099f634cbf18ef7c to 6ccffea5cae20acd255f3712f6ba8f5e7ab35ebc
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
9137595 | Fix minor issues in function field docstrings
|
a6852c5 | cleanup code and docstrings
|
bd18422 | Merge branch 'develop' into t/16576/ticket/16576
|
5daae9a | fix typos
|
aa5b887 | Merge branch 'u/saraedum/ticket/16563' in 8.0.b8
|
eebe2c8 | Merge branch 'develop' into t/16563/public/16563
|
9aca052 | Merge branch 't/16563/public/16563' into t/16575/ticket/16575
|
4cce86f | Merge branch 't/16575/ticket/16575' into t/16576/ticket/16576
|
4460407 | Merge branch 't/16576/ticket/16576' into t/16562/ticket/16562
|
6ccffea | Merge branch 't/16562/ticket/16562' into t/16564/ticket/16564
|
comment:15 Changed 5 years ago by
- Commit changed from 6ccffea5cae20acd255f3712f6ba8f5e7ab35ebc to 303e2c64d210628be07ea39ce8f3cfcff05c1451
Branch pushed to git repo; I updated commit sha1. New commits:
303e2c6 | removed merge artifacts
|
comment:16 Changed 5 years ago by
- Commit changed from 303e2c64d210628be07ea39ce8f3cfcff05c1451 to 05d66cd5277fbeecb051b1b7c42b4a703c57c029
Branch pushed to git repo; I updated commit sha1. New commits:
4bba9ae | is_separable method added to function field class
|
444fb59 | INPUT and OUTPUT blocks added to documentation.
|
3cbf43f | Fix docbuilding errors
|
05d66cd | Merge remote-tracking branch 'trac/u/saraedum/add__is_separable__to_function_field_class' into t/16564/ticket/16564
|
comment:17 Changed 5 years ago by
- Commit changed from 05d66cd5277fbeecb051b1b7c42b4a703c57c029 to 6ee8eee7e408b04ba65a201f224fa61a52d95a0e
Branch pushed to git repo; I updated commit sha1. New commits:
6ee8eee | fix relative imports
|
comment:18 Changed 5 years ago by
- Dependencies changed from #16561, #16562 to #16561, #16562, #23152
- Status changed from needs_work to needs_review
comment:19 Changed 4 years ago by
- Branch u/saraedum/ticket/16564 deleted
- Commit 6ee8eee7e408b04ba65a201f224fa61a52d95a0e deleted
comment:20 Changed 4 years ago by
- Branch set to u/saraedum/16564
comment:21 Changed 4 years ago by
- Commit set to beed9377bfeabadbff3548425780a8e9b5fdd37f
- Work issues BuildFailed, merge conflict deleted
Last 10 new commits:
cb3a16b | Merge branch 'u/saraedum/ticket/16562' of git://trac.sagemath.org/sage into t/16564/ticket/16564
|
256f7cf | Fixed doctests which failed due to changed printing of function fields and their morphisms
|
f594422 | Merge branch 't/16576/ticket/16576' into t/16562/ticket/16562
|
9645e83 | Added forgotten . before constructor.
|
4460407 | Merge branch 't/16576/ticket/16576' into t/16562/ticket/16562
|
6ccffea | Merge branch 't/16562/ticket/16562' into t/16564/ticket/16564
|
303e2c6 | removed merge artifacts
|
05d66cd | Merge remote-tracking branch 'trac/u/saraedum/add__is_separable__to_function_field_class' into t/16564/ticket/16564
|
6ee8eee | fix relative imports
|
beed937 | Merge remote-tracking branch 'trac/u/saraedum/ticket/16564' into 16564
|
comment:22 Changed 4 years ago by
- Commit changed from beed9377bfeabadbff3548425780a8e9b5fdd37f to 3816bd9c2790857e8f06a1b9c7b27da05c315bf3
Branch pushed to git repo; I updated commit sha1. New commits:
3816bd9 | Merge remote-tracking branch 'trac/develop' into 16564
|
comment:23 Changed 4 years ago by
- Work issues set to is the patchbot happy?
comment:24 Changed 4 years ago by
- Commit changed from 3816bd9c2790857e8f06a1b9c7b27da05c315bf3 to 1f3aebe358e26f3532858f0d15ac9cea4bb4a0b7
Branch pushed to git repo; I updated commit sha1. New commits:
1f3aebe | Change is_separable() to be recursive
|
comment:25 Changed 4 years ago by
- Commit changed from 1f3aebe358e26f3532858f0d15ac9cea4bb4a0b7 to 3816bd9c2790857e8f06a1b9c7b27da05c315bf3
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
comment:26 Changed 4 years ago by
- Status changed from needs_review to needs_work
- Work issues changed from is the patchbot happy? to lots of failing doctests
Changed 4 years ago by
comment:27 Changed 4 years ago by
Changing FunctionFieldIsomorphism
to FunctionFieldVectorSpaceIsomorphism
in line 424 of function_field/maps.py
yields a working version. All that remains are some small failures in doctests, which I have attached.
comment:28 Changed 4 years ago by
- Commit changed from 3816bd9c2790857e8f06a1b9c7b27da05c315bf3 to 682fb8053557115ac9c0387bae3a10fcf5eceedc
Branch pushed to git repo; I updated commit sha1. New commits:
682fb80 | Merge remote-tracking branch 'trac/develop' into 16564
|
comment:29 Changed 4 years ago by
Thanks, that renaming was not intentional. The printing of morphisms has changed, so these errors are expected.
comment:30 Changed 4 years ago by
- Commit changed from 682fb8053557115ac9c0387bae3a10fcf5eceedc to 6ea2e69184fb591d5deaf14ad4cfc17a6c21d1c1
comment:31 Changed 4 years ago by
- Status changed from needs_work to needs_review
- Work issues changed from lots of failing doctests to is the patchbot happy?
comment:32 Changed 4 years ago by
- Status changed from needs_review to needs_work
This builds for me along with its documentation, and looks pretty great. Here are some remarks.
(1) In the method _test_derivation
you may also want to test that d(f)
is zero, or more precisely that if we calculate d(f)
term by term we also get 0. Or yet differently, the lift to a derivation in both x and y should send f to zero.
(2) Similarly, you may want to check that the deriviation is non-trivial.
(3) In the docstring, it seems better to use k
for the field of constants instead of K
, simply for consistency, and leave K
to be the function field in question. You may also want to point out that you do not assume K
to be the function field of an absolutely irreducible curve. (This is why you need k
to be perfect.)
In the first part of the description of the algorithm, you may want to write "a rational function field F
over k
" instead of "another function field F
", and in the complementary case you can mention that this is where you use the hypothesis on k
being perfect.
Points (1) and (2) are not very important; indeed, I performed done thousands of ad hoc tests of them without a single error, and the algorithms are mathematically correct as far as I can judge. Only the points in (3) apply. They are very small and apart from them this addition seems completely neat.
comment:33 Changed 4 years ago by
Sorry, I messed up this fancy terminology: where I wrote "absolutely irreducible" I should have written "geometrically reduced", or perhaps "geometrically integral".
comment:34 Changed 4 years ago by
sijsling thanks for the review. If you want to make these changes yourself, please feel free to make them and I'll review. Otherwise, I'll hopfefully get around to make the necessary changes myself soon.
comment:35 Changed 4 years ago by
- Branch changed from u/saraedum/16564 to u/sijsling/16564
- Commit 6ea2e69184fb591d5deaf14ad4cfc17a6c21d1c1 deleted
comment:36 Changed 4 years ago by
- Commit set to 3a07e3424e13dd42f668ef817db197fec6b3f87b
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
6ccffea | Merge branch 't/16562/ticket/16562' into t/16564/ticket/16564
|
303e2c6 | removed merge artifacts
|
05d66cd | Merge remote-tracking branch 'trac/u/saraedum/add__is_separable__to_function_field_class' into t/16564/ticket/16564
|
6ee8eee | fix relative imports
|
beed937 | Merge remote-tracking branch 'trac/u/saraedum/ticket/16564' into 16564
|
3816bd9 | Merge remote-tracking branch 'trac/develop' into 16564
|
682fb80 | Merge remote-tracking branch 'trac/develop' into 16564
|
268984f | Fix incorrect merge in 3816bd9c2790857e8f06a1b9c7b27da05c315bf3
|
6ea2e69 | Adapt doctest output to changed printing
|
3a07e34 | Improve documentation and tests
|
comment:37 Changed 4 years ago by
- Status changed from needs_work to needs_review
comment:38 Changed 4 years ago by
- Branch changed from u/sijsling/16564 to u/saraedum/16564
comment:39 Changed 4 years ago by
- Commit changed from 3a07e3424e13dd42f668ef817db197fec6b3f87b to f63c4fff210aef4bbec4d9cce99bd2dc46ab0fbd
- Reviewers set to Jeroen Sijsling, Julian Rüth
- Work issues changed from is the patchbot happy? to is the patchbot happy? ⇒ positive review
Thanks, I made a minimal change to the unit test code. If your happy with this and doctests pass, then we can set this to positive review :)
New commits:
f63c4ff | Do not assert in tests
|
comment:40 Changed 4 years ago by
I am of course fine with that! Rebuilt just to be sure and of course everything still worked. So now it is up to the bot to see if there are other issues.
comment:41 Changed 4 years ago by
The pyflakes complaints are not bugs and are fine anyway with Python 3 as list comprehensions do not leak variables anymore.
The failing doctest in sage.databases.sql_db.SQLDatabase.__init__
can't really be caused by the changes here.
comment:42 Changed 3 years ago by
- Reviewers changed from Jeroen Sijsling, Julian Rüth to Jeroen Sijsling, Julian Rüth, Stefan Wewers
- Status changed from needs_review to positive_review
- Work issues is the patchbot happy? ⇒ positive review deleted
As other have said before: the change are all fine, and it would be very nice to have them available soon. The problems with the patchbot seem to be unrelated to this ticket.
comment:43 Changed 3 years ago by
- Dependencies #16561, #16562, #23152 deleted
- Milestone changed from sage-6.4 to sage-8.7
comment:44 Changed 3 years ago by
- Branch changed from u/saraedum/16564 to f63c4fff210aef4bbec4d9cce99bd2dc46ab0fbd
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
Merge branch 'ticket/16561' into ticket/16564
Merge branch 'develop' into ticket/16523
Merge branch 'ticket/16523' into ticket/16563
Merge branch 'ticket/16563' into ticket/16562
Merge branch 'ticket/16563' into ticket/16575
Merge branch 'ticket/16575' into ticket/16576
Merge branch 'ticket/16576' into ticket/16562
minor docstring fix
Merge branch 'ticket/16563' into ticket/16562
Merge branch 'ticket/16562' into ticket/16564