Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A framework for discrete valuations in Sage #21869

Closed
saraedum opened this issue Nov 12, 2016 · 479 comments
Closed

A framework for discrete valuations in Sage #21869

saraedum opened this issue Nov 12, 2016 · 479 comments

Comments

@saraedum
Copy link
Member

Based on the sage package https://github.com/mclf/mac_lane.

Depends on #21782
Depends on #23166
Depends on #23167
Depends on #21879
Depends on #23185
Depends on #23203
Depends on #23204
Depends on #23211
Depends on #23188
Depends on #21879
Depends on #23186
Depends on #23191
Depends on #21996
Depends on #23190
Depends on #23495
Depends on #23483
Depends on #23510
Depends on #23525
Depends on #23620
Depends on #23642
Depends on #23965
Depends on #23966

Component: commutative algebra

Keywords: discrete valuations, valuations, function fields, smooth projective curves, Mac Lane algorithm, Montes algorithm, sd87

Author: Julian Rüth

Branch: 24807e3

Reviewer: GaYee Park, Stefan Wewers, David Roe, Padmavathi Srinivasan, Shiva Chidambaram

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

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@saraedum

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 16, 2018

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

e01a3abRequire incomparability

@saraedum
Copy link
Member Author

comment:294

The changes could make some outputs gain precision. Let's see what the patchbots think…

@roed314
Copy link
Contributor

roed314 commented Jan 16, 2018

comment:295

There's an extra space in the new commit: 10*x^1 2.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 16, 2018

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

2142891fix copy & paste mistake

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 16, 2018

Changed commit from e01a3ab to 2142891

@roed314
Copy link
Contributor

roed314 commented Jan 16, 2018

comment:297

I'm trying to build the documentation now to check the previous change.


New commits:

2142891fix copy & paste mistake

@saraedum
Copy link
Member Author

comment:298
File "src/sage/rings/padics/padic_valuation.py", line 726, in sage.rings.padics.padic_valuation.pAdicValuation_base.extensions
Failed example:
    QQ.valuation(2).extensions(L)
Expected:
    Traceback (most recent call last):
    ...
    ValueError: The valuation [ Gauss valuation induced by 2-adic valuation, v(x) = 1/2 ] does not approximate a unique extension of 2-adic valuation with respect to x^4 + 2*x^3 + 2*x^2 + 8
Got:
    [[ 2-adic valuation, v(x + 2) = 3/2 ]-adic valuation,
     [ 2-adic valuation, v(x) = 1/2 ]-adic valuation]

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2018

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

963ceb7Fix incorrect doctest output
4900f56Merge branch 'u/saraedum/a_framework_for_discrete_valuations_in_sage' of git://trac.sagemath.org/sage into t/21869/a_framework_for_discrete_valuations_in_sage

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2018

Changed commit from 2142891 to 4900f56

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2018

Changed commit from 4900f56 to 24807e3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2018

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

24807e3Merge remote-tracking branch 'trac/develop' into t/21869/a_framework_for_discrete_valuations_in_sage

@saraedum

This comment has been minimized.

@roed314
Copy link
Contributor

roed314 commented Feb 8, 2018

comment:303

The failing builds look to be due to some unrelated giac issue, and I don't know what the failed apply is. I'm happy with the changes and the documentation builds when I test it. Let's set this back to positive review.

@vbraun
Copy link
Member

vbraun commented Feb 9, 2018

@saraedum
Copy link
Member Author

saraedum commented Mar 6, 2018

Changed commit from 24807e3 to none

@saraedum

This comment has been minimized.

@jdemeyer
Copy link

comment:306

What's the point of these doctests?

        Note that this does not affect comparison of valuations which do not
        coerce into a common parent. This is by design in Sage, see
        :meth:`sage.structure.element.Element.__richcmp__`. When the valuations
        do not coerce into a common parent, a rather random comparison of
        ``id`` happens::

            sage: w = valuations.TrivialValuation(GF(2))
            sage: w <= v # random output
            True
            sage: v <= w # random output
            False

I plan to remove them in #26934

@saraedum
Copy link
Member Author

comment:307

I thought that the docstring is quite technical, so the doctest illustrates what I mean. They don't "test" anything, they are really part of the documentation.

@jdemeyer
Copy link

comment:308

Replying to @saraedum:

I thought that the docstring is quite technical, so the doctest illustrates what I mean.

I see your point, but it's not the job of valuation.py to document technical details of the coercion model. I ask because I plan to change the behaviour of comparisons in #22029.

@saraedum
Copy link
Member Author

comment:309

It's a detail of the coercion model but here the point is just that it does not work the way you would expect it to. Comparisons are important in this context and it's therefore important to document the limitations. If it raises an error, all the better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants