Opened 7 years ago
Closed 3 years ago
#16564 closed enhancement (fixed)
Derivations for inseparable function fields
Reported by:  saraedum  Owned by:  

Priority:  minor  Milestone:  sage8.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 7 years ago by
 Keywords sd59 added
comment:2 Changed 7 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 7 years ago by
 Status changed from new to needs_review
comment:4 Changed 7 years ago by
 Commit set to 1e32daed9b798cecf0fe399b072f4ec55fdd2e26
comment:5 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:6 Changed 6 years ago by
 Status changed from needs_review to needs_work
 Work issues set to BuildFailed
comment:7 Changed 6 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 6 years ago by
 Status changed from needs_work to needs_review
comment:9 Changed 6 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 5 years ago by
 Status changed from needs_review to needs_work
comment:11 Changed 4 years ago by
 Keywords sd86.5 added
comment:12 Changed 4 years ago by
 Work issues changed from BuildFailed to BuildFailed, merge conflict
comment:13 Changed 4 years ago by
comment:14 Changed 4 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 4 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 4 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 remotetracking branch 'trac/u/saraedum/add__is_separable__to_function_field_class' into t/16564/ticket/16564

comment:17 Changed 4 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 4 years ago by
 Dependencies changed from #16561, #16562 to #16561, #16562, #23152
 Status changed from needs_work to needs_review
comment:19 Changed 3 years ago by
 Branch u/saraedum/ticket/16564 deleted
 Commit 6ee8eee7e408b04ba65a201f224fa61a52d95a0e deleted
comment:20 Changed 3 years ago by
 Branch set to u/saraedum/16564
comment:21 Changed 3 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 remotetracking branch 'trac/u/saraedum/add__is_separable__to_function_field_class' into t/16564/ticket/16564

6ee8eee  fix relative imports

beed937  Merge remotetracking branch 'trac/u/saraedum/ticket/16564' into 16564

comment:22 Changed 3 years ago by
 Commit changed from beed9377bfeabadbff3548425780a8e9b5fdd37f to 3816bd9c2790857e8f06a1b9c7b27da05c315bf3
Branch pushed to git repo; I updated commit sha1. New commits:
3816bd9  Merge remotetracking branch 'trac/develop' into 16564

comment:23 Changed 3 years ago by
 Work issues set to is the patchbot happy?
comment:24 Changed 3 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 3 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 3 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 3 years ago by
comment:27 Changed 3 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 3 years ago by
 Commit changed from 3816bd9c2790857e8f06a1b9c7b27da05c315bf3 to 682fb8053557115ac9c0387bae3a10fcf5eceedc
Branch pushed to git repo; I updated commit sha1. New commits:
682fb80  Merge remotetracking branch 'trac/develop' into 16564

comment:29 Changed 3 years ago by
Thanks, that renaming was not intentional. The printing of morphisms has changed, so these errors are expected.
comment:30 Changed 3 years ago by
 Commit changed from 682fb8053557115ac9c0387bae3a10fcf5eceedc to 6ea2e69184fb591d5deaf14ad4cfc17a6c21d1c1
comment:31 Changed 3 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 3 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 nontrivial.
(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 3 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 3 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 3 years ago by
 Branch changed from u/saraedum/16564 to u/sijsling/16564
 Commit 6ea2e69184fb591d5deaf14ad4cfc17a6c21d1c1 deleted
comment:36 Changed 3 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 remotetracking branch 'trac/u/saraedum/add__is_separable__to_function_field_class' into t/16564/ticket/16564

6ee8eee  fix relative imports

beed937  Merge remotetracking branch 'trac/u/saraedum/ticket/16564' into 16564

3816bd9  Merge remotetracking branch 'trac/develop' into 16564

682fb80  Merge remotetracking 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 3 years ago by
 Status changed from needs_work to needs_review
comment:38 Changed 3 years ago by
 Branch changed from u/sijsling/16564 to u/saraedum/16564
comment:39 Changed 3 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 3 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 3 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 sage6.4 to sage8.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