Opened 5 years ago
Closed 4 years ago
#25607 closed enhancement (fixed)
Shrink coefficients in Mac Lane algorithm
Reported by: | saraedum | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-8.3 |
Component: | padics | Keywords: | |
Cc: | swewers | Merged in: | |
Authors: | Julian Rüth | Reviewers: | Stefan Wewers |
Report Upstream: | N/A | Work issues: | |
Branch: | 8d2324a (Commits, GitHub, GitLab) | Commit: | 8d2324a89be21300206687a265833b5db6e6609f |
Dependencies: | #25397 | Stopgaps: |
Description (last modified by )
As a followup to #25397, let's try to shrink coefficients even further, using the ideas from https://github.com/MCLF/mclf/pull/60/files.
This also speeds up things overall. All the valuation tests took 406s on my machine before and now take 314s.
Change History (15)
comment:1 Changed 5 years ago by
Branch: | → u/saraedum/25607 |
---|
comment:2 Changed 5 years ago by
Branch: | u/saraedum/25607 |
---|---|
Dependencies: | → #25397 |
comment:3 Changed 5 years ago by
Branch: | → u/saredum/25607 |
---|
comment:4 Changed 5 years ago by
Branch: | u/saredum/25607 → u/saraedum/25607 |
---|---|
Commit: | → c1af2bb1ac3c47c3b60c9b40c9d527c4e644d269 |
comment:5 Changed 5 years ago by
Commit: | c1af2bb1ac3c47c3b60c9b40c9d527c4e644d269 → e71ad336d242ec20bcdbab23d0ea17435bb78c7e |
---|
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5890fdd | Better element_with_valuation for function fields
|
59221ba | Implement restrictions/element_with_valuation in more places
|
f87b794 | Merge remote-tracking branch 'trac/develop' into 25397
|
076b369 | Merge remote-tracking branch 'trac/develop' into 25397
|
3592f74 | Merge branch '25397' into 25607
|
e71ad33 | Shrink coefficients in valuations
|
comment:6 Changed 5 years ago by
Description: | modified (diff) |
---|---|
Status: | new → needs_review |
Work issues: | → is the patchbot happy? |
comment:7 Changed 5 years ago by
swewers: If you want to review this (I hope you do ;) This is really a somewhat wild mix of all kinds of ideas and little errors that I found. I can split it up into more isolated bits if you think it's overwhelming like that.
In the end I did not really take any new ideas from your pull request. I think that everything that's in there already existed (just some things did not work/were not called.) If you have any additional ideas in how to improve this, I would be very interested of course :)
comment:8 Changed 5 years ago by
Btw., the doctests in mclf
still works with this. Very few outputs change because the centers of affinoids are now sightly simpler.
Failed example: Y.etale_locus() Expected: Affinoid with 3 components: Elementary affinoid defined by v(x) >= 3/4 Elementary affinoid defined by v(x + 7) >= 5/4 Elementary affinoid defined by v(x + 2) >= 5/4 Got: Affinoid with 3 components: Elementary affinoid defined by v(x) >= 3/4 Elementary affinoid defined by v(x - 2) >= 5/4 Elementary affinoid defined by v(x + 2) >= 5/4 <BLANKLINE>
comment:11 Changed 5 years ago by
Reviewers: | → Stefan Wewers |
---|---|
Status: | positive_review → needs_work |
Soem pyflakes errors.
comment:12 Changed 5 years ago by
Status: | needs_work → positive_review |
---|---|
Work issues: | is the patchbot happy? |
The pyflakes errors are already fixed in another ticket that touches padic_valuation. Let's ignore them here to avoid merge conflicts.
comment:13 Changed 4 years ago by
Commit: | e71ad336d242ec20bcdbab23d0ea17435bb78c7e → 8d2324a89be21300206687a265833b5db6e6609f |
---|---|
Status: | positive_review → needs_review |
comment:14 Changed 4 years ago by
Status: | needs_review → positive_review |
---|
just merged in develop. back to positive review.
comment:15 Changed 4 years ago by
Branch: | u/saraedum/25607 → 8d2324a89be21300206687a265833b5db6e6609f |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
New commits:
WIP