sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.43k stars 479 forks source link

A framework for discrete valuations in Sage #21869

Closed saraedum closed 6 years ago

saraedum commented 7 years ago

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 commented 7 years ago

Description changed:

--- 
+++ 
@@ -1 +1,26 @@
 This is a meta-ticket to keep track of the progress of integrating https://github.com/saraedum/sage/releases into Sage.
+
+## Fix bugs in Sage
+There are a number of trivial bugs that get fixed by monkey-patches in https://github.com/saraedum/sage/blob/experimental/mac_lane/__init__.py
+
+1. Conversion from a Function Field to its Constant Field
+2. Conversion from a Function Field to its underlying Polynomial Ring
+3. Coercions between Function Fields
+4. Coercions are injective if the underlying map is
+5. Ring homomorphisms from Fields are injective
+6. The embedding of a ring into a polynomial ring over that ring is injective
+7. Morphisms of number fields are injective
+8. ZZ into QQ is injective
+9. ZZ into a Number Field is injective
+10. ZZ into an order of a Number Field is injective
+11. (some_elements() should return more than just [1] for most rings.)
+
+## Add new features to Sage
+New features that the code needs to work
+
+1. Factorization over iterated extensions of finite fields.
+2. principal_part() and sides() of a Newton Polygon
+
+## Add the valuation code to Sage
+
+i.e., add these files The files here https://github.com/saraedum/sage/tree/experimental/mac_lane to Sage.
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -23,4 +23,4 @@

 ## Add the valuation code to Sage

-i.e., add these files The files here https://github.com/saraedum/sage/tree/experimental/mac_lane to Sage.
+i.e., add these files https://github.com/saraedum/sage/tree/experimental/mac_lane to Sage.
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -3,7 +3,7 @@
 ## Fix bugs in Sage
 There are a number of trivial bugs that get fixed by monkey-patches in https://github.com/saraedum/sage/blob/experimental/mac_lane/__init__.py

-1. Conversion from a Function Field to its Constant Field
+1. Conversion from a Function Field to its Constant Field #21872
 2. Conversion from a Function Field to its underlying Polynomial Ring
 3. Coercions between Function Fields
 4. Coercions are injective if the underlying map is
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -6,13 +6,13 @@
 1. Conversion from a Function Field to its Constant Field #21872
 2. Conversion from a Function Field to its underlying Polynomial Ring
 3. Coercions between Function Fields
-4. Coercions are injective if the underlying map is
-5. Ring homomorphisms from Fields are injective
+4. Coercions are injective if the underlying map is #21879
+5. Ring homomorphisms from Fields are injective #21879
 6. The embedding of a ring into a polynomial ring over that ring is injective
-7. Morphisms of number fields are injective
-8. ZZ into QQ is injective
-9. ZZ into a Number Field is injective
-10. ZZ into an order of a Number Field is injective
+7. Morphisms of number fields are injective #21879
+8. ZZ into QQ is injective #21879
+9. ZZ into a Number Field is injective #21879
+10. ZZ into an order of a Number Field is injective #21879
 11. (some_elements() should return more than just [1] for most rings.)

 ## Add new features to Sage
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -13,7 +13,8 @@
 8. ZZ into QQ is injective #21879
 9. ZZ into a Number Field is injective #21879
 10. ZZ into an order of a Number Field is injective #21879
-11. (some_elements() should return more than just [1] for most rings.)
+11. ZpCA shifts are broken
+12. (some_elements() should return more than just [1] for most rings.)

 ## Add new features to Sage
 New features that the code needs to work
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -13,7 +13,7 @@
 8. ZZ into QQ is injective #21879
 9. ZZ into a Number Field is injective #21879
 10. ZZ into an order of a Number Field is injective #21879
-11. ZpCA shifts are broken
+11. ~~ZpCA shifts are broken~~
 12. (some_elements() should return more than just [1] for most rings.)

 ## Add new features to Sage
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -19,7 +19,7 @@
 ## Add new features to Sage
 New features that the code needs to work

-1. Factorization over iterated extensions of finite fields.
+1. Factorization over iterated extensions of finite fields. #21996
 2. principal_part() and sides() of a Newton Polygon

 ## Add the valuation code to Sage
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -21,6 +21,7 @@

 1. Factorization over iterated extensions of finite fields. #21996
 2. principal_part() and sides() of a Newton Polygon
+3. (cached_in_argument_method #22034)

 ## Add the valuation code to Sage
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-This is a meta-ticket to keep track of the progress of integrating https://github.com/saraedum/sage/releases into Sage.
+This is a meta-ticket to keep track of the progress of integrating https://github.com/saraedum/mac_lane into Sage.

 ## Fix bugs in Sage
 There are a number of trivial bugs that get fixed by monkey-patches in https://github.com/saraedum/sage/blob/experimental/mac_lane/__init__.py
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -1,7 +1,7 @@
 This is a meta-ticket to keep track of the progress of integrating https://github.com/saraedum/mac_lane into Sage.

 ## Fix bugs in Sage
-There are a number of trivial bugs that get fixed by monkey-patches in https://github.com/saraedum/sage/blob/experimental/mac_lane/__init__.py
+There are a number of trivial bugs that get fixed by monkey-patches in https://github.com/saraedum/mac_lane/blob/master/__init__.py

 1. Conversion from a Function Field to its Constant Field #21872
 2. Conversion from a Function Field to its underlying Polynomial Ring
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -4,7 +4,7 @@
 There are a number of trivial bugs that get fixed by monkey-patches in https://github.com/saraedum/mac_lane/blob/master/__init__.py

 1. Conversion from a Function Field to its Constant Field #21872
-2. Conversion from a Function Field to its underlying Polynomial Ring
+2. Conversion from a Function Field to its underlying Polynomial Ring #23166
 3. Coercions between Function Fields
 4. Coercions are injective if the underlying map is #21879
 5. Ring homomorphisms from Fields are injective #21879
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -5,7 +5,7 @@

 1. Conversion from a Function Field to its Constant Field #21872
 2. Conversion from a Function Field to its underlying Polynomial Ring #23166
-3. Coercions between Function Fields
+3. Coercions between Function Fields #23167
 4. Coercions are injective if the underlying map is #21879
 5. Ring homomorphisms from Fields are injective #21879
 6. The embedding of a ring into a polynomial ring over that ring is injective
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -25,4 +25,4 @@

 ## Add the valuation code to Sage

-i.e., add these files https://github.com/saraedum/sage/tree/experimental/mac_lane to Sage.
+i.e., add these files https://github.com/saraedum/mac_lane to Sage.
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -20,8 +20,8 @@
 New features that the code needs to work

 1. Factorization over iterated extensions of finite fields. #21996
-2. principal_part() and sides() of a Newton Polygon
-3. (cached_in_argument_method #22034)
+~1. principal_part() and sides() of a Newton Polygon~ (patch this in the calling code instead.)
+1. (cached_in_argument_method #22034)

 ## Add the valuation code to Sage
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -20,8 +20,8 @@
 New features that the code needs to work

 1. Factorization over iterated extensions of finite fields. #21996
-~1. principal_part() and sides() of a Newton Polygon~ (patch this in the calling code instead.)
-1. (cached_in_argument_method #22034)
+2. ~~principal_part() and sides() of a Newton Polygon~~ (patch this in the calling code instead.)
+3. (cached_in_argument_method #22034)

 ## Add the valuation code to Sage
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -8,13 +8,14 @@
 3. Coercions between Function Fields #23167
 4. Coercions are injective if the underlying map is #21879
 5. Ring homomorphisms from Fields are injective #21879
-6. The embedding of a ring into a polynomial ring over that ring is injective
-7. Morphisms of number fields are injective #21879
-8. ZZ into QQ is injective #21879
-9. ZZ into a Number Field is injective #21879
-10. ZZ into an order of a Number Field is injective #21879
-11. ~~ZpCA shifts are broken~~
-12. (some_elements() should return more than just [1] for most rings.)
+6. Polynomial rings embed into their fraction fields #23185
+7. The embedding of a ring into a polynomial ring over that ring is injective
+8. Morphisms of number fields are injective #21879
+9. ZZ into QQ is injective #21879
+10. ZZ into a Number Field is injective #21879
+11. ZZ into an order of a Number Field is injective #21879
+12. ~~ZpCA shifts are broken~~
+13. (some_elements() should return more than just [1] for most rings.)

 ## Add new features to Sage
 New features that the code needs to work
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -14,8 +14,9 @@
 9. ZZ into QQ is injective #21879
 10. ZZ into a Number Field is injective #21879
 11. ZZ into an order of a Number Field is injective #21879
-12. ~~ZpCA shifts are broken~~
-13. (some_elements() should return more than just [1] for most rings.)
+12. ZZ does not map onto QQ #23186
+13. ~~ZpCA shifts are broken~~
+14. (some_elements() should return more than just [1] for most rings.)

 ## Add new features to Sage
 New features that the code needs to work
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -9,14 +9,15 @@
 4. Coercions are injective if the underlying map is #21879
 5. Ring homomorphisms from Fields are injective #21879
 6. Polynomial rings embed into their fraction fields #23185
-7. The embedding of a ring into a polynomial ring over that ring is injective
-8. Morphisms of number fields are injective #21879
-9. ZZ into QQ is injective #21879
-10. ZZ into a Number Field is injective #21879
-11. ZZ into an order of a Number Field is injective #21879
-12. ZZ does not map onto QQ #23186
-13. ~~ZpCA shifts are broken~~
-14. (some_elements() should return more than just [1] for most rings.)
+7. ~~The embedding of a ring into a polynomial ring over that ring is injective~~
+8. p-adic rings embed into their fraction fields #23188
+9. Morphisms of number fields are injective #21879
+10. ZZ into QQ is injective #21879
+11. ZZ into a Number Field is injective #21879
+12. ZZ into an order of a Number Field is injective #21879
+13. ZZ does not map onto QQ #23186
+14. ~~ZpCA shifts are broken~~
+15. (some_elements() should return more than just [1] for most rings.)

 ## Add new features to Sage
 New features that the code needs to work
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -13,11 +13,12 @@
 8. p-adic rings embed into their fraction fields #23188
 9. Morphisms of number fields are injective #21879
 10. ZZ into QQ is injective #21879
-11. ZZ into a Number Field is injective #21879
-12. ZZ into an order of a Number Field is injective #21879
-13. ZZ does not map onto QQ #23186
-14. ~~ZpCA shifts are broken~~
-15. (some_elements() should return more than just [1] for most rings.)
+11. quotients of polynomial rings are injective/surjective #23190
+12. ZZ into a Number Field is injective #21879
+13. ZZ into an order of a Number Field is injective #21879
+14. ZZ does not map onto QQ #23186
+15. ~~ZpCA shifts are broken~~
+16. (some_elements() should return more than just [1] for most rings.)

 ## Add new features to Sage
 New features that the code needs to work
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -27,6 +27,12 @@
 2. ~~principal_part() and sides() of a Newton Polygon~~ (patch this in the calling code instead.)
 3. (cached_in_argument_method #22034)

+## Make tests non-trivial
+
+1. (some_elements() should be non-trivial for number fields/orders)
+2. (some_elements() should be non-trivial for rational function fields and their extensions)
+3. (some_elements() should be non-trivial for fraction_fields of polynomial rings)
+
 ## Add the valuation code to Sage

 i.e., add these files https://github.com/saraedum/mac_lane to Sage.
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -30,7 +30,7 @@
 ## Make tests non-trivial

 1. (some_elements() should be non-trivial for number fields/orders)
-2. (some_elements() should be non-trivial for rational function fields and their extensions)
+2. (some_elements() should be non-trivial/deterministic for rational function fields and their extensions)
 3. (some_elements() should be non-trivial for fraction_fields of polynomial rings)

 ## Add the valuation code to Sage
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -18,7 +18,8 @@
 13. ZZ into an order of a Number Field is injective #21879
 14. ZZ does not map onto QQ #23186
 15. ~~ZpCA shifts are broken~~
-16. (some_elements() should return more than just [1] for most rings.)
+16. add default implementation of inverse_of_unit()  #23191
+17. (some_elements() should return more than just [1] for most rings.)

 ## Add new features to Sage
 New features that the code needs to work
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -19,7 +19,6 @@
 14. ZZ does not map onto QQ #23186
 15. ~~ZpCA shifts are broken~~
 16. add default implementation of inverse_of_unit()  #23191
-17. (some_elements() should return more than just [1] for most rings.)

 ## Add new features to Sage
 New features that the code needs to work
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -29,9 +29,9 @@

 ## Make tests non-trivial

-1. (some_elements() should be non-trivial for number fields/orders)
-2. (some_elements() should be non-trivial/deterministic for rational function fields and their extensions)
-3. (some_elements() should be non-trivial for fraction_fields of polynomial rings)
+1. (some_elements() should be non-trivial for number fields/orders) #23192
+2. (some_elements() should be non-trivial/deterministic for rational function fields and their extensions) #23193
+3. (some_elements() should be non-trivial for fraction_fields of polynomial rings) #23194

 ## Add the valuation code to Sage
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -9,7 +9,7 @@
 4. Coercions are injective if the underlying map is #21879
 5. Ring homomorphisms from Fields are injective #21879
 6. Polynomial rings embed into their fraction fields #23185
-7. ~~The embedding of a ring into a polynomial ring over that ring is injective~~
+7. The embedding of a ring into a polynomial ring over that ring is injective #23203
 8. p-adic rings embed into their fraction fields #23188
 9. Morphisms of number fields are injective #21879
 10. ZZ into QQ is injective #21879
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -9,7 +9,7 @@
 4. Coercions are injective if the underlying map is #21879
 5. Ring homomorphisms from Fields are injective #21879
 6. Polynomial rings embed into their fraction fields #23185
-7. The embedding of a ring into a polynomial ring over that ring is injective #23203
+7. The embedding of a ring into a polynomial ring over that ring is injective #23203, #23204, #23211
 8. p-adic rings embed into their fraction fields #23188
 9. Morphisms of number fields are injective #21879
 10. ZZ into QQ is injective #21879
saraedum commented 7 years ago

Branch: u/saraedum/a_framework_for_discrete_valuations_in_sage

roed314 commented 7 years ago

Changed keywords from discrete valuations, valuations, function fields, smooth projective curves, Mac Lane algorithm, Montes algorithm to discrete valuations, valuations, function fields, smooth projective curves, Mac Lane algorithm, Montes algorithm, sd87

roed314 commented 7 years ago

Last 10 new commits:

925d8fafix typo in comment
c4a7b1aAdded a tutorial in the README
20e6110move to subdirectory for merging with sage tree
ed206ecremoving gitignore for merge with sage tree
14799b0Merge mac_lane infrastructure for discrete valuations into sage
0767201remove mac_lane LICENSE
bd1dcbfremove obsolete TODOs
0eb3fffremove monkey patches
6c3a301move valuation code to valuation/
0f615c7remove specific valuation code out of valuation/
roed314 commented 7 years ago

Commit: 0f615c7

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

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

8afc361remove standalone import commands
4153ef9Wired up valuations in the Sage library
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 0f615c7 to 4153ef9

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 4153ef9 to c0a81c8

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

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

c0a81c8fix function lookup
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -1,4 +1,10 @@
 This is a meta-ticket to keep track of the progress of integrating https://github.com/saraedum/mac_lane into Sage.
+
+# Review
+
+For your convenience you can review this ticket at https://github.com/saraedum/mac_lane/pull/4 (and leave inline comments.)
+
+# Necessary changes

 ## Fix bugs in Sage
 There are a number of trivial bugs that get fixed by monkey-patches in https://github.com/saraedum/mac_lane/blob/master/__init__.py
saraedum commented 7 years ago

New commits:

c0a81c8fix function lookup
saraedum commented 7 years ago

Work Issues: move references to references file, move README to sage documentation, make sure that valuation() has a lot of the documentation and the factory just references it, remove optional: integrated bits

saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -3,6 +3,29 @@
 # Review

 For your convenience you can review this ticket at https://github.com/saraedum/mac_lane/pull/4 (and leave inline comments.)
+
+Please check off `[x]` the following when you think that a file is in good shape (modulo the comments that you made.) Or put a `[-]` if it needs substantial work.
+
+```
+[ ] function_field/function_field_valuation.py
+[ ] padics/discrete_value_group.py
+[ ] padics/padic_valuation.py
+[-] valuation/README.md
+[x] valuation/__init__.py
+[ ] valuation/all.py
+[ ] valuation/augmented_valuation.py
+[ ] valuation/developing_valuation.py
+[ ] valuation/gauss_valuation.py
+[ ] valuation/inductive_valuation.py
+[ ] valuation/limit_valuation.py
+[ ] valuation/mapped_valuation.py
+[ ] valuation/scaled_valuation.py
+[ ] valuation/trivial_valuation.py
+[ ] valuation/valuation.py
+[ ] valuation/valuation_space.py
+[ ] valuation/valuations_catalog.py
+[ ] valuation/value_group.py
+```

 # Necessary changes
saraedum commented 7 years ago

Author: Julian Rüth

saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -4,7 +4,7 @@

 For your convenience you can review this ticket at https://github.com/saraedum/mac_lane/pull/4 (and leave inline comments.)

-Please check off `[x]` the following when you think that a file is in good shape (modulo the comments that you made.) Or put a `[-]` if it needs substantial work.
+Please check off `[x]` the following when you think that a file is in good shape (modulo the comments that you made.) Or put a `[-]` if it needs substantial work. You can put your name next to file to tell others that you are already having a look at it.

[ ] function_field/function_field_valuation.py @@ -22,7 +22,7 @@ [ ] valuation/scaled_valuation.py [ ] valuation/trivial_valuation.py [ ] valuation/valuation.py -[ ] valuation/valuation_space.py +[ ] valuation/valuation_space.py (Padmavathi) [ ] valuation/valuations_catalog.py [ ] valuation/value_group.py

saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -22,9 +22,9 @@
 [ ] valuation/scaled_valuation.py
 [ ] valuation/trivial_valuation.py
 [ ] valuation/valuation.py
-[ ] valuation/valuation_space.py (Padmavathi)
+[ ] valuation/valuation_space.py
 [ ] valuation/valuations_catalog.py
-[ ] valuation/value_group.py
+[ ] valuation/value_group.py (Padmavathi)

Necessary changes

e0d76a0f-4cc0-4362-be8d-c0832f95b471 commented 7 years ago
comment:36

I was reviewing gauss_valuation.py and tried reducing a polynomial using the Gauss valuation induced by the 2-adic valuation, and got an unexpected error message. I thought it would tell me that the polynomial wasn't integral, and instead it gave me a coercion error.

sage: v
Gauss valuation induced by 2-adic valuation
sage: v.domain()
Univariate Polynomial Ring in y over Integer Ring
sage: h
1/2*y^2
sage: v.reduce(h)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-108-4d78566b468b> in <module>()
----> 1 v.reduce(h)

/projects/da1818ed-996d-4de6-acc6-361415b7725d/user/padma_sk/mac_lane/gauss_valuation.py in reduce(self, f, check, degree_bound, coefficients, valuations)
    360
    361         """
--> 362         f = self.domain().coerce(f)
    363
    364         if degree_bound is not None:

/usr/local/sage/src/sage/structure/parent.pyx in sage.structure.parent.Parent.coerce (/usr/local/sage/src/build/cythonized/sage/structure/parent.c:11229)()
   1166             return False
   1167
-> 1168     cpdef coerce(self, x):
   1169         """
   1170         Return x as an element of self, if and only if there is a canonical

/usr/local/sage/src/sage/structure/parent.pyx in sage.structure.parent.Parent.coerce (/usr/local/sage/src/build/cythonized/sage/structure/parent.c:11158)()
   1193                 except Exception:
   1194                     _record_exception()
-> 1195             raise TypeError("no canonical coercion from %s to %s" % (parent(x), self))
   1196         else:
   1197             return (<map.Map>mor)._call_(x)

TypeError: no canonical coercion from Univariate Polynomial Ring in y over Rational Field to Univariate Polynomial Ring in y over Integer Ring
e0d76a0f-4cc0-4362-be8d-c0832f95b471 commented 7 years ago
comment:37

equivalence_unit from gauss_valuation.py isn't outputting the results shown in the example input -2 as shown in the file.

sage: v
Gauss valuation induced by 2-adic valuation
sage: v.domain()
Univariate Polynomial Ring in y over Integer Ring
sage: v.equivalence_unit(2)
4
sage: v.equivalence_unit(-2)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-128-71aadb857663> in <module>()
----> 1 v.equivalence_unit(-Integer(2))

/usr/local/sage/src/sage/misc/cachefunc.pyx in sage.misc.cachefunc.CachedMethodCaller.__call__ (/usr/local/sage/src/build/cythonized/sage/misc/cachefunc.c:1079
2)()
   2036                 return cache[k]
   2037         except KeyError:
-> 2038             w = self._instance_call(*args, **kwds)
   2039             cache[k] = w
   2040             return w

/usr/local/sage/src/sage/misc/cachefunc.pyx in sage.misc.cachefunc.CachedMethodCaller._instance_call (/usr/local/sage/src/build/cythonized/sage/misc/cachefunc.
c:10238)()
   1912             True
   1913         """
-> 1914         return self.f(self._instance, *args, **kwds)
   1915
   1916     cdef fix_args_kwds(self, tuple args, dict kwds):

/projects/da1818ed-996d-4de6-acc6-361415b7725d/user/padma_sk/mac_lane/gauss_valuation.py in equivalence_unit(self, s, reciprocal)
    474             return self.equivalence_reciprocal(self.equivalence_unit(-s))
    475
--> 476         ret = self._base_valuation.element_with_valuation(s)
    477         return self.domain()(ret)
    478

/projects/da1818ed-996d-4de6-acc6-361415b7725d/user/padma_sk/mac_lane/valuation_space.py in element_with_valuation(self, s)
    465             s = QQ.coerce(s)
    466             if s not in self.value_semigroup():
--> 467                 raise ValueError("s must be in the value semigroup of this valuation but %r is not in %r"%(s, self.value_semigroup()))
    468             if s == 0:
    469                 return self.domain().one()

ValueError: s must be in the value semigroup of this valuation but -2 is not in Additive Abelian Semigroup generated by 1
saraedum commented 7 years ago
comment:38

Thanks for reporting this. I think this is ok. The problem here is not that the polynomial is not integral but that the polynomial is not in the domain.

This works:

sage: R.<x> = QQ[]
sage: v = valuations.GaussValuation(R, QQ.valuation(2))
sage: v.reduce(x + 1/2)
ValueError: reduction not defined for non-integral elements and x + 1/2 is not integral over Gauss valuation induced by 2-adic valuation

This does not:

sage: R.<x> = ZZ[]
sage: v = valuations.GaussValuation(R, ZZ.valuation(2))
sage: v.reduce(x + 1/2)
TypeError: no canonical coercion from Univariate Polynomial Ring in x over Rational Field to Univariate Polynomial Ring in x over Integer Ring

Replying to @sagetrac-padma-sk:

I was reviewing gauss_valuation.py and tried reducing a polynomial using the Gauss valuation induced by the 2-adic valuation, and got an unexpected error message. I thought it would tell me that the polynomial wasn't integral, and instead it gave me a coercion error.

sage: v
Gauss valuation induced by 2-adic valuation
sage: v.domain()
Univariate Polynomial Ring in y over Integer Ring
sage: h
1/2*y^2
sage: v.reduce(h)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-108-4d78566b468b> in <module>()
----> 1 v.reduce(h)

/projects/da1818ed-996d-4de6-acc6-361415b7725d/user/padma_sk/mac_lane/gauss_valuation.py in reduce(self, f, check, degree_bound, coefficients, valuations)
    360
    361         """
--> 362         f = self.domain().coerce(f)
    363
    364         if degree_bound is not None:

/usr/local/sage/src/sage/structure/parent.pyx in sage.structure.parent.Parent.coerce (/usr/local/sage/src/build/cythonized/sage/structure/parent.c:11229)()
   1166             return False
   1167
-> 1168     cpdef coerce(self, x):
   1169         """
   1170         Return x as an element of self, if and only if there is a canonical

/usr/local/sage/src/sage/structure/parent.pyx in sage.structure.parent.Parent.coerce (/usr/local/sage/src/build/cythonized/sage/structure/parent.c:11158)()
   1193                 except Exception:
   1194                     _record_exception()
-> 1195             raise TypeError("no canonical coercion from %s to %s" % (parent(x), self))
   1196         else:
   1197             return (<map.Map>mor)._call_(x)

TypeError: no canonical coercion from Univariate Polynomial Ring in y over Rational Field to Univariate Polynomial Ring in y over Integer Ring
saraedum commented 7 years ago
comment:39

You are in the wrong domain in this example I think: This is the error you are seeing because there is no element of valuation -2 in ZZ[x].

sage: R.<x> = ZZ[]
sage: v = valuations.GaussValuation(R, ZZ.valuation(2))
sage: v.element_with_valuation(-2)
ValueError: s must be in the value semigroup of this valuation but -2 is not in Additive Abelian Semigroup generated by 1

However, this works in QQ[x].

sage: sage: R.<x> = QQ[]
sage: v = valuations.GaussValuation(R, QQ.valuation(2))
sage: v.element_with_valuation(-2)
1/4

Does that make sense? (Should it be documented more explicitly?)

Replying to @sagetrac-padma-sk:

equivalence_unit from gauss_valuation.py isn't outputting the results shown in the example input -2 as shown in the file.

sage: v
Gauss valuation induced by 2-adic valuation
sage: v.domain()
Univariate Polynomial Ring in y over Integer Ring
sage: v.equivalence_unit(2)
4
sage: v.equivalence_unit(-2)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-128-71aadb857663> in <module>()
----> 1 v.equivalence_unit(-Integer(2))

/usr/local/sage/src/sage/misc/cachefunc.pyx in sage.misc.cachefunc.CachedMethodCaller.__call__ (/usr/local/sage/src/build/cythonized/sage/misc/cachefunc.c:1079
2)()
   2036                 return cache[k]
   2037         except KeyError:
-> 2038             w = self._instance_call(*args, **kwds)
   2039             cache[k] = w
   2040             return w

/usr/local/sage/src/sage/misc/cachefunc.pyx in sage.misc.cachefunc.CachedMethodCaller._instance_call (/usr/local/sage/src/build/cythonized/sage/misc/cachefunc.
c:10238)()
   1912             True
   1913         """
-> 1914         return self.f(self._instance, *args, **kwds)
   1915
   1916     cdef fix_args_kwds(self, tuple args, dict kwds):

/projects/da1818ed-996d-4de6-acc6-361415b7725d/user/padma_sk/mac_lane/gauss_valuation.py in equivalence_unit(self, s, reciprocal)
    474             return self.equivalence_reciprocal(self.equivalence_unit(-s))
    475
--> 476         ret = self._base_valuation.element_with_valuation(s)
    477         return self.domain()(ret)
    478

/projects/da1818ed-996d-4de6-acc6-361415b7725d/user/padma_sk/mac_lane/valuation_space.py in element_with_valuation(self, s)
    465             s = QQ.coerce(s)
    466             if s not in self.value_semigroup():
--> 467                 raise ValueError("s must be in the value semigroup of this valuation but %r is not in %r"%(s, self.value_semigroup()))
    468             if s == 0:
    469                 return self.domain().one()

ValueError: s must be in the value semigroup of this valuation but -2 is not in Additive Abelian Semigroup generated by 1
saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -15,7 +15,7 @@
 [ ] valuation/all.py
 [ ] valuation/augmented_valuation.py
 [ ] valuation/developing_valuation.py
-[ ] valuation/gauss_valuation.py
+[ ] valuation/gauss_valuation.py (Padmavathi)
 [ ] valuation/inductive_valuation.py
 [ ] valuation/limit_valuation.py
 [ ] valuation/mapped_valuation.py
saraedum commented 7 years ago

Dependencies: #21782, #23166, #23167, #21879, #23185, #23203, #23204, #23211, #23188, #21879, #23186, #23191, #21996

saraedum commented 7 years ago

Changed dependencies from #21782, #23166, #23167, #21879, #23185, #23203, #23204, #23211, #23188, #21879, #23186, #23191, #21996 to #21782, #23166, #23167, #21879, #23185, #23203, #23204, #23211, #23188, #21879, #23186, #23191, #21996, #23190

e0d76a0f-4cc0-4362-be8d-c0832f95b471 commented 7 years ago

Description changed:

--- 
+++ 
@@ -13,7 +13,7 @@
 [-] valuation/README.md
 [x] valuation/__init__.py
 [ ] valuation/all.py
-[ ] valuation/augmented_valuation.py
+[ ] valuation/augmented_valuation.py (Padmavathi)
 [ ] valuation/developing_valuation.py
 [ ] valuation/gauss_valuation.py (Padmavathi)
 [ ] valuation/inductive_valuation.py
e0d76a0f-4cc0-4362-be8d-c0832f95b471 commented 7 years ago

Description changed:

--- 
+++ 
@@ -13,10 +13,10 @@
 [-] valuation/README.md
 [x] valuation/__init__.py
 [ ] valuation/all.py
-[ ] valuation/augmented_valuation.py (Padmavathi)
+[ ] valuation/augmented_valuation.py 
 [ ] valuation/developing_valuation.py
 [ ] valuation/gauss_valuation.py (Padmavathi)
-[ ] valuation/inductive_valuation.py
+[ ] valuation/inductive_valuation.py (Padmavathi)
 [ ] valuation/limit_valuation.py
 [ ] valuation/mapped_valuation.py
 [ ] valuation/scaled_valuation.py