Opened 2 months ago

Closed 11 days ago

#23497 closed enhancement (fixed)

relocate dynamical systems code from sage/schemes to sage/dynamics

Reported by: bhutz Owned by:
Priority: major Milestone: sage-8.1
Component: dynamics Keywords: sd87, days88, IMA coding sprints
Cc: xander.faber, tscrim Merged in:
Authors: Ben Hutz, Xander Faber Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 2eecc68 (Commits) Commit: 2eecc68b05527c59db06daba1bedadc5756e8e05
Dependencies: Stopgaps:

Description (last modified by bhutz)

the majority of the code for working with arithmetical dynamical systems is in the respective morphism files in the sage/schemes folder. This is not very intuitive and as the code grows this is becoming increasing less ideal.

This ticket proposes creating a sage/dynamics/arithmetic_dynamics folder to house the dynamical systems code. This includes creating a class to distinguish between SchemeMorphism_polynomials which happen to be endomorphisms and those which should be treated dynamical systems. I consider this extra class as the negative effect of moving the code.

There are really two main reasons for this move.

  • the dynamical systems functionality is extensive enough that it should be separated from the schemes functionality. There is already a sage/dynamics folders so it makes sense to put it there.
  • creating a dynamical system under the new structure is much easier and intuitive for the user

Change History (46)

comment:1 Changed 2 months ago by bhutz

  • Authors set to Ben Hutz
  • Branch set to u/bhutz/arith_dyn
  • Commit set to b4d09658f577eb9be7d232e5ef25a310c330c028

I've acheived an initial move of the code that achieves 'all tests pass', so I figured that was a good place to commit and get another set of eyes on this.

There remains much clean-up to be done. A few things I haven't done yet

  • do we need to specify domain=P to get point iteration to work? What works and what doesn't with the domains
  • is .as_scheme_morphism and .as_dynamical_system a performance hit (and excess object creation in memory)? If so, what's a better way to deal with this (basically the parent of a dynamical system is the endomorphism ring of schemes which creates a schememorphism...)
  • dynamicalsystems_generic defaults to affine which probably should be projective
  • some more intelligence in the creation functions

New commits:

b4d096523497: relocate arithmetic dynamics code: initial version

comment:2 Changed 2 months ago by xander.faber

  • Branch changed from u/bhutz/arith_dyn to u/xander.faber/arith_dyn

comment:3 Changed 2 months ago by xander.faber

  • Commit changed from b4d09658f577eb9be7d232e5ef25a310c330c028 to 4950fcc87670d8a7625b31627e3ceae1cca86b48
  • Summary changed from relocate dyanmical systems code from sage/schemes to sage/dynamics to relocate dynamical systems code from sage/schemes to sage/dynamics

New commits:

0a40209Fixed a docstring in morphism.py
f6827d0AffineSpace now accepts a univariate polynomial ring and constructs A^1
586f17fImproved dynamical system constructors in generic_ds module
4950fccStreamlined dynamical system class init's after pushing hard work to the constructors in generic_ds module

comment:4 Changed 2 months ago by git

  • Commit changed from 4950fcc87670d8a7625b31627e3ceae1cca86b48 to b9327f83f69e74707e3dc597ba19ec4f9b79b57d

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

a018703Fixed a coercion bug in the affine morphism constructor
4650eb3Improved consistency and type checking in class constructors
b9327f8Fixed argument handling in special class inits for efficiency

comment:5 Changed 2 months ago by bhutz

  • Branch changed from u/xander.faber/arith_dyn to u/bhutz/arith_dyn

comment:6 Changed 2 months ago by bhutz

  • Commit changed from b9327f83f69e74707e3dc597ba19ec4f9b79b57d to 296dccdc9efe4c8b48e69cb10c4799bd94e6fa9e

I consider this ready for a review now.

When we are happy with this new functionality, then we'll need to merge in the following tickets which will conflict: #23234, #23333, #23334. So don't mark it positive-reivew even if it is ready.


New commits:

f9f291c23497: reinstate doctests for deprecated functions
ae729ce23497: tests, docs, and init updates to generic_ds.py
296dccd23497: finish other dynamics files and all docs

comment:7 Changed 8 weeks ago by git

  • Commit changed from 296dccdc9efe4c8b48e69cb10c4799bd94e6fa9e to 2c604d062974dfb5241b8027ca06c0ce5c0cd2fe

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

2c604d0Merge branch 8.1.beta0 into t/23497/arith_dyn

comment:8 Changed 7 weeks ago by xander.faber

Build succeeds, as do all doctests. I walked through the documentation and some examples of the new constructors ... looks pretty solid.

comment:9 Changed 7 weeks ago by chapoton

The branch is red above, meaning that it does not apply on the latest beta (8.1.b1).

comment:10 Changed 7 weeks ago by bhutz

Yes, I'm aware. This probably also conflicts with a number of tickets currently positive-review. I plan to do all that merging today now that the functionality for this ticket is stable.

comment:11 Changed 7 weeks ago by git

  • Commit changed from 2c604d062974dfb5241b8027ca06c0ce5c0cd2fe to ba22abdc1b2d85f716c0512fcb7b0ec86fb8dbf1

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

ba22abdMerge branch 8.1.beta1 into t/23497/arith_dyn

comment:12 Changed 7 weeks ago by bhutz

  • Status changed from new to needs_review

All the tickets I was concerned about conflicting were already in 8.1.beta1, so this is ready now that I've resolved the conflict.

comment:13 Changed 5 weeks ago by chapoton

  • Status changed from needs_review to needs_work

does not apply

comment:14 Changed 5 weeks ago by tscrim

  • Cc tscrim added
  • Reviewers set to Travis Scrimshaw

If you do one more rebase, I will quickly review it (so hopefully there will not be another conflict).

comment:15 Changed 5 weeks ago by bhutz

great, thanks. I was going to do this today as soon as I finish the rebase for a different ticket.

comment:16 Changed 5 weeks ago by git

  • Commit changed from ba22abdc1b2d85f716c0512fcb7b0ec86fb8dbf1 to 79378c30d23f4c3da284272115634e3776ef8c81

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

79378c3Merge branch 8.1.beta3 into t/23497/arith_dyn

comment:17 Changed 5 weeks ago by bhutz

  • Authors changed from Ben Hutz to Ben Hutz, Xander Faber
  • Status changed from needs_work to needs_review

merge conflict fixed


New commits:

79378c3Merge branch 8.1.beta3 into t/23497/arith_dyn

comment:18 Changed 5 weeks ago by tscrim

  • Keywords days88 IMA coding sprints added
  • Status changed from needs_review to needs_work

Comments. I'm sorry if some of this is coming from old code already in Sage, but this might be a good time to clean it if it does.

Can you explain a little more about what is in the description about the extra class and why it is necessary? I don't really know this code/math so much to fully understand.

Indentation here:

.. [FH2015]     \J. A. de Faria, B. Hutz. Combinatorics of Cycle Lengths on
            Wehler K3 Surfaces over finite fields. New Zealand Journal
            of Mathematics 45 (2015), 19–31.

Do you want to worry about backwards compatibility with imports?

-class DynamicalSystem_affine_ring(SchemeMorphism_polynomial_affine_space,\
-                             DynamicalSystem_generic):
+class DynamicalSystem_affine_ring(SchemeMorphism_polynomial_affine_space,
+                                  DynamicalSystem_generic):

and similar; in particular, you do not need the \.

     INPUT:
 
-    - ``polys_or_rat_fncts`` -- a list of ``n`` polynomials or rational
-      functions, all of which should have the same parent.
+      functions, all of which should have the same parent
-
-    - ``domain`` -- an affine scheme embedded in `\mathbb{A}^n`.
+    - ``domain`` -- an affine scheme embedded in `\mathbb{A}^n`
     def __init__(self, polys_or_rat_fncts, domain):
         r"""
         The Python constructor.
 
         See :class:`DynamicalSystem_generic` for details.
-
-        INPUT:
-
-        - ``polys_or_rat_fncts`` -- a list of polynomials or rational functions.
-
-        - ``domain`` -- the domain of the map to be constructed.
-
-        OUTPUT:
-
-        - :class:`DynamicalSystem_affine`.
         INPUT:
 
-        - ``n`` -- a tuple of nonnegative integers. If ``n`` is an integer,
-          then the two values of the tuple are assumed to be the same.
+        - ``n`` -- a tuple of nonnegative integers; if ``n`` is an integer,
+          then the two values of the tuple are assumed to be the same

and similar changes.

+Return the ``n``-th iterate of self.
-Return the ``n``-th iterate of ``self``.
-       INPUT: ``n`` - a positive integer.
+       INPUT:
+
+       - ``n`` -- a positive integer

In, e.g., orbit, do not use self as a latex variable. Instead say something like, "let X be a dynamical system."

Returns -> Return and similar for one-line descriptions.

You are using a number of different formats for the OUTPUT:. I think you should at least be consistent, if not match the Sage coding conventions.

Remove is_DynamicalSystem because the isinstance already does what you want.

This is done in a number of other files.

from sage.categories.fields import Fields
_Fields = Fields()

Although I guess since module is lazily imported, it is not such a big deal. However, I would be happy to see not making this perma-instance issue worse, but not a strict requirement for a positive review.

-    Return a dynamical system on a projective scheme
+    Return a dynamical system on a projective scheme.
+    We can define dynamical systems on P^1 by giving a polynomial or
-    We can define dynamical systems on `P^1` by giving a polynomial or
     rational function::

Missing a doctest:

class DynamicalSystem_generic(SchemeMorphism_polynomial):
    def __init__(self, polys_or_rat_fncts, domain):
-        REFERENCES: [Hutz2015]_, [MoPa1994]_
+        REFERENCES:
+
+        - [Hutz2015]_
+        - [MoPa1994]_
             sage: dyn.parent()
             Symbolic Ring
-       """
+        """
         if self.domain().ngens() > 2:
             raise TypeError("does not make sense in dimension >1")

Error messages should start with a lower case and not end with a period/full stop.

-        - ``N`` -- iterate to use for approximation (optional - default: 3).
-
-        - ``prec`` -- real precision to use when computing root (optional - default: 53).
+        - ``N`` -- (default: 3) iterate to use for approximation
+        - ``prec`` -- (default: 53) real precision to use when computing root

(The extra whitespace can be added back in; that is your preference.)

             sage: RSA768 = 123018668453011775513049495838496272077285356959533479219732245215\
-                1726400507263657518745202199786469389956474942774063845925192557326303453731548\
-                2685079170261221429134616704292143116022212404792747377940806653514195974598569\
-                02143413
             ....: 1726400507263657518745202199786469389956474942774063845925192557326303453731548\
             ....: 2685079170261221429134616704292143116022212404792747377940806653514195974598569\
             ....: 02143413

Deindent:

        ALGORITHM:

            Uses a Nullstellensatz argument to compute the constant.
            For details: see [Hutz2015]_.
        ALGORITHM:
            Calls ``self.possible_periods()`` modulo all primes of good reduction in range
            ``prime_bound``. Returns the intersection of those lists.

In a number of those functions that you now are deprecating, they are not calling the new functions, correct? I think they should simply call those functions they say they are being deprecated in favor of. Some of them do this, but not all of them I believe.

There might be more comments later, but this is what I have for now on my first pass.

comment:19 Changed 5 weeks ago by bhutz

  • Description modified (diff)

responding to a couple of the questions:

  • the only thing new here is the dynamicalsystem class and the structure needed to create that class. The functionality within the new class is *all* relocated code. All the deprecated functions in schemes should essentially just call the new functions. I'd recommend not trying to review every single line of the relocated code for the member functions, but am happy to do any clean-up needed as you come across it.
  • There are really two main reasons for this move.
    • the dynamical systems functionality is extensive enough that it should be separated from the schemes functionality. There is already a sage/dynamics folders so it makes sense to put it there.
    • creating a dynamical system under the new structure is much easier and intuitive for the user

comment:20 Changed 5 weeks ago by git

  • Commit changed from 79378c30d23f4c3da284272115634e3776ef8c81 to 80ff2e673f93c961618923251d8d3d19786ad015

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

80ff2e623497: some fixes from review

comment:21 follow-up: Changed 5 weeks ago by bhutz

  • Status changed from needs_work to needs_review

I fixed most of those and have some discussion for the others.

  • Note sure what you mean with the _Fields() import. I was told a couple years ago that it really should be done this way. Has that changed? What are the arguments for and against?
  • the INPUT and OUTPUT consistently end in a '.' throughout the dynamical systems and schemes files, so I've left that consistent. However, the developer's guide does seem to give examples without ending in a period.
  • For the INPUT you are suggesting the default be moved to the beginning on the description. Right now dynamical systems and schemes are consistently the other way. The conventions seem only to mention that the default value should be specified, not where.
  • I'm not sure what you mean for the OUTPUT consistency. Yes, the OUTPUT description is formatted differently based on what makes the most sense for the OUTPUT of the functions, but they all match the conventions in the developer's guide.
  • I disagree with removing is_DynamicalSystem. isinstance only works after you import the DynamicalSystem_generic class. With the distinction between endomorphisms and dynamical systems, it is important for the user to have an easy check for what they are working with.
  • I couldn't find any instances of whether the error had capital or ending punctuation. Where did you see that?

So, the big open issue I see is whether to go through dynamical systems and schemes and change all the INPUT/OUTPUT formatting. What do you think?


New commits:

80ff2e623497: some fixes from review

comment:22 Changed 5 weeks ago by bhutz

to answer the other question. No, I'm not worried about backwards compatibility. I think keeping the old functionality working through deprecation is sufficient.

comment:23 in reply to: ↑ 21 Changed 5 weeks ago by tscrim

Replying to bhutz:

  • Note sure what you mean with the _Fields() import. I was told a couple years ago that it really should be done this way. Has that changed? What are the arguments for and against?

You create a _Fields object in the global namespace, not an import, so it essentially becomes permanent in memory (although it is not a true memory leak). That is the argument against it. The argument for is that you might be creating the Fields() category frequently, which can slow things down. However, this only occurs when creating objects, so I would rather just replace _Fields with Fields(). Actually, IIRC, you can just do containment checking by X in Fields (without creating/getting an instance object).

The comment about the lazy import is that this is not done on startup, but only when you create one of these dynamical system objects.

  • the INPUT and OUTPUT consistently end in a '.' throughout the dynamical systems and schemes files, so I've left that consistent. However, the developer's guide does seem to give examples without ending in a period.

The convention is without periods and this is what most of Sage does.

  • For the INPUT you are suggesting the default be moved to the beginning on the description. Right now dynamical systems and schemes are consistently the other way. The conventions seem only to mention that the default value should be specified, not where.

I believe what most of Sage does is put it first, and this is what I do with all of my code that I write and request when I review. However I don't strongly care.

  • I'm not sure what you mean for the OUTPUT consistency. Yes, the OUTPUT description is formatted differently based on what makes the most sense for the OUTPUT of the functions, but they all match the conventions in the developer's guide.

Here are some of the styles used:

OUTPUT: some output
OUTPUT: some output.
OUTPUT:

- A full sentence.
OUTPUT:

A full sentence.
OUTPUT:

- a short description
  • I disagree with removing is_DynamicalSystem. isinstance only works after you import the DynamicalSystem_generic class. With the distinction between endomorphisms and dynamical systems, it is important for the user to have an easy check for what they are working with.

Almost none of the is_* methods are imported into the global namespace. In fact, there are 2 (is_field is a little special). So is_DynamicalSystem should not be imported, and hence someone will have to import something anyways. Moreover, if someone wants to know the class that they are working with, then they really should be checking the appropriate class if they don't want to rely on ducktyping or trusting the user.

  • I couldn't find any instances of whether the error had capital or ending punctuation. Where did you see that?

Hmmm...I might have mixed up tickets or misremembered the Symbolic Ring errors.

So, the big open issue I see is whether to go through dynamical systems and schemes and change all the INPUT/OUTPUT formatting. What do you think?

I wasn't sure how much you were actually adding versus copying over. Thank you for clarifying that. I am okay with letting it pass because it is how it was, but it would be nice to start converting it to the new version. I can do this with a reviewer change too.

comment:24 follow-up: Changed 5 weeks ago by bhutz

I'll remove the global _Fields().

I'm still not 100% convinced that is_DynamicalSystem is not needed. Here is my thought process in some more detail: One of the big issues we struggled with on this ticket is how to differentiate between an endomorphism of a space and a dynamical system on a space without creating a category of schemes whose arrows are dynamical systems instead of scheme morphisms. Let's say

f = End(P)

g = DynamicalSystem?(...) # on P

Then f.parent() == g.parent() is true. So without is_DynSys the only way to tell the difference is to check the class or see what member functions are available. There are simple conversion functions; e.g. f.as_dynamical_system(). Looking at the global is_* namespace, there are 21 functions currently. (The two is_ProductProjectiveSpaces is_ProjectiveSpace seem out of place and perhaps should be removed from the global name space as part of the schemes clean-up.) To me, it is a question of will a standard user need this function or not. A power user writing their own code should be able to do imports, but a standard user should not have to import something they need. I am having some trouble coming up with a realistic scenario where a standard user absolutely needs is_DynSys. Let me think about it some more.

For the INPUT/OUTPUT formatting: Updating the files in sage/dynamical systems/arithmetic dynamics/ is certainly a reasonable request as they are created here and I can do that (probably later today). I think I'd prefer updating the sage/schemes/ files on a different ticket.

comment:25 in reply to: ↑ 24 Changed 5 weeks ago by tscrim

Replying to bhutz:

I'll remove the global _Fields().

If this slows things down, feel free to leave it in. Like I said, it's not a big issue because this module lazily imported.

I'm still not 100% convinced that is_DynamicalSystem is not needed. Here is my thought process in some more detail: One of the big issues we struggled with on this ticket is how to differentiate between an endomorphism of a space and a dynamical system on a space without creating a category of schemes whose arrows are dynamical systems instead of scheme morphisms. Let's say

f = End(P)

g = DynamicalSystem?(...) # on P

Then f.parent() == g.parent() is true. So without is_DynSys the only way to tell the difference is to check the class or see what member functions are available. There are simple conversion functions; e.g. f.as_dynamical_system(). Looking at the global is_* namespace, there are 21 functions currently. (The two is_ProductProjectiveSpaces is_ProjectiveSpace seem out of place and perhaps should be removed from the global name space as part of the schemes clean-up.) To me, it is a question of will a standard user need this function or not. A power user writing their own code should be able to do imports, but a standard user should not have to import something they need. I am having some trouble coming up with a realistic scenario where a standard user absolutely needs is_DynSys. Let me think about it some more.

I think most people will first try:

if isinstance(X, DynamicalSystem):
    pass

which, of course, will be False because DynamicalSystem is a function. What I usually do for constructors that mimic ABCs is to actually fuse the two and use __classcall__. I really like this idiom as it better associates the (abstract) class and the constructor. So I would rename DynamicalSystem_generic to DynamicalSystem, which then has a __classcall_private__ (which you would also need to set the metaclass to be ClasscallMetaclass AFAICS) that constructs the appropriate subclass.

However, this problem is no different than other homsets that have different elements/morphisms that use different subclasses. I doubt a standard user will be writing code that mixes the two or needs to differentiate the two classes based upon what you've told me as they would be working solely in the dynamical system perspective if that is what they want (especially since it is a subclass).

For the INPUT/OUTPUT formatting: Updating the files in sage/dynamical systems/arithmetic dynamics/ is certainly a reasonable request as they are created here and I can do that (probably later today). I think I'd prefer updating the sage/schemes/ files on a different ticket.

Sounds good to me. Let me know if you want me to do it as well.

comment:26 Changed 5 weeks ago by bhutz

I've done the INPUT/OUTPUT formatting, but I'm having trouble with the classcall_private. I was able to get it to work for dynamicalsystems_generic since the classcall function is basically just deciding if it is affine or projective and then calling the appropriate class constructor.

I was trying to also do DS_affine and DS_projective the same way. However, I can't seem to get from the _classcall_private_ function over to the _init_ function. Some of the examples I found in Sage seem to have a secondary class so that classcall_private calls the constructor of the subclass. I guess I could do the same here, but is this really how it should work? I'd end up with

dynamcalsystem_affine (with _classcall_private_) which would then call the constructor for one of the subclasses DS_affine_ring, DS_affine_field, or DS_affine_finite_field. In other words, basically a dummy higher class to have _classcall_private_ which calls the actual class constructors. It seems like I'm not understanding how this should all work.

Do you know how _classcall_private_ should interact with _init_?

comment:27 Changed 5 weeks ago by tscrim

So __clascall_private__ takes input and then it will be passed up with a super(Foo, cls).__classcall__(cls, args) call (as in other examples, e.g., Partition). This then calls the __init__ with these methods. (Just incase you are unaware, this needs to be an @staticmethod.) However, if you want to delegate to, e.g., a subclass (but it really can be anything), you do return Foo(bar).

Maybe a more standard example that just standardizes input would be algebras/yokonuma_hecke_algebra.py (note that the metaclass is set because it is a subclass of UniqueRepresentation).

comment:28 Changed 5 weeks ago by tscrim

I can do this tomorrow morning when I wake up if you want.

comment:29 Changed 5 weeks ago by git

  • Commit changed from 80ff2e673f93c961618923251d8d3d19786ad015 to cc50108eb32d1fc33d030cc128eecbf6747d67a9

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

fdf726523497: doc formatting INPUT/OUTPUT
cc5010823497: use __classcall_private__

comment:30 Changed 5 weeks ago by bhutz

I tried that first as that is what most instances are doing, but I get an AttributeError? that super doesn't have a _classcall_. If you could take a look that would be appreciated.

I've pushed the INPUT/OUTPUT clean-up as well as setting up the changes needed to use _classcall_private_. It currently has the super()._classcall_() where I think it should go. If you can get that initialization to work and push that back up, I'll go through and fix any remaining issues since I couldn't really test as it is now.

comment:31 Changed 5 weeks ago by tscrim

  • Branch changed from u/bhutz/arith_dyn to u/tscrim/arith_dyn-23497
  • Commit changed from cc50108eb32d1fc33d030cc128eecbf6747d67a9 to 7408d79a2c8833d0c496cc32a3755f1405bc2151

Okay, I've fix up the classcall. It didn't have a superclass that actually implemented the __classcall__, so you have to explicitly have to use typecall. I forgot about that, sorry. I also made it through the doc and code and did a bunch of cleanup and more micro-optimizations I know.

If my changes look good, then positive review.


New commits:

4bbdcdeFixing things with __classcall_private__.
7408d79Cleaning up the doc and improving the code.

comment:32 Changed 5 weeks ago by bhutz

  • Branch changed from u/tscrim/arith_dyn-23497 to u/bhutz/arith_dyn-23497

comment:33 Changed 5 weeks ago by bhutz

  • Commit changed from 7408d79a2c8833d0c496cc32a3755f1405bc2151 to d00cea258b3b5797ca06ddd33ac55bf4e6ae701c
  • Status changed from needs_review to positive_review

Thanks! I had just a couple minor fixes.


New commits:

d00cea223497: a couple minor fixes

comment:34 Changed 5 weeks ago by tscrim

No problem. Thanks for making the initial round of fixes and adapting the code.

comment:35 Changed 4 weeks ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:36 Changed 3 weeks ago by git

  • Commit changed from d00cea258b3b5797ca06ddd33ac55bf4e6ae701c to 300a3b1fe5773b7983736cbbed23ed13c15e20b7

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

300a3b1Merge branch 8.1.beta4 into t/23497/arith_dyn-23497

comment:37 Changed 3 weeks ago by bhutz

  • Status changed from needs_work to needs_review

conflict resolved

comment:38 Changed 3 weeks ago by tscrim

  • Status changed from needs_review to positive_review

comment:39 Changed 3 weeks ago by fbissey

  • Status changed from positive_review to needs_work

doctest_warning -> doctest:warning

sage -t --long /usr/lib64/python2.7/site-packages/sage/schemes/projective/projective_morphism.py
**********************************************************************
File "/usr/lib64/python2.7/site-packages/sage/schemes/projective/projective_morphism.py", line 1754, in sage.schemes.projective.projective_morphism.SchemeMorphism_polynomial_projective_space_field.rational_periodic_points
Failed example:
    sorted(f.rational_periodic_points(prime_bound=20, lifting_prime=7)) # long time
Expected:
    doctest_warning
    ...
    [(-1/2 : 1), (1 : 0), (3/2 : 1)]
Got:
    doctest:warning
      File "/usr/lib/python-exec/python2.7/sage-runtests", line 114, in <module>
        err = DC.run()
      File "/usr/lib64/python2.7/site-packages/sage/doctest/control.py", line 1099, in run
        self.run_doctests()
      File "/usr/lib64/python2.7/site-packages/sage/doctest/control.py", line 824, in run_doctests
        self.dispatcher.dispatch()
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 1777, in dispatch
        self.parallel_dispatch()
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 1667, in parallel_dispatch
        w.start()  # This might take some time
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 1943, in start
        super(DocTestWorker, self).start()
      File "/usr/lib64/python2.7/multiprocessing/process.py", line 130, in start
        self._popen = Popen(self)
      File "/usr/lib64/python2.7/multiprocessing/forking.py", line 126, in __init__
        code = process_obj._bootstrap()
      File "/usr/lib64/python2.7/multiprocessing/process.py", line 258, in _bootstrap
        self.run()
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 1916, in run
        task(self.options, self.outtmpfile, msgpipe, self.result_queue)
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 2218, in __call__
        result = runner.run(test)
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 663, in run
        return self._run(test, compileflags, out)
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 518, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 888, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.schemes.projective.projective_morphism.SchemeMorphism_polynomial_projective_space_field.rational_periodic_points[3]>", line 1, in <module>
        sorted(f.rational_periodic_points(prime_bound=Integer(20), lifting_prime=Integer(7))) # long time
      File "/usr/lib64/python2.7/site-packages/sage/schemes/projective/projective_morphism.py", line 1760, in rational_periodic_points
        deprecation(23479, "use sage.dynamics.arithmetic_dynamics.projective_ds.rational_periodic_points instead")
      File "/usr/lib64/python2.7/site-packages/sage/misc/superseded.py", line 101, in deprecation
        warning(trac_number, message, DeprecationWarning, stacklevel)
      File "/usr/lib64/python2.7/site-packages/sage/misc/superseded.py", line 142, in warning
        warn(message, warning_class, stacklevel)
    :
    DeprecationWarning: use sage.dynamics.arithmetic_dynamics.projective_ds.rational_periodic_points instead
    See http://trac.sagemath.org/23479 for details.
    [(-1/2 : 1), (1 : 0), (3/2 : 1)]
**********************************************************************

Considering the size of the branch, having a just a small typo like that as a mistake is not bad.

comment:40 Changed 3 weeks ago by tscrim

  • Branch changed from u/bhutz/arith_dyn-23497 to u/tscrim/arith_dyn-23497
  • Commit changed from 300a3b1fe5773b7983736cbbed23ed13c15e20b7 to bdbf658dcbb2005227228a13330d5d4090d33724
  • Status changed from needs_work to needs_review

Thanks for catching that François. Here's a branch with the fix.


New commits:

bdbf658Fixing typo in doctest.

comment:41 Changed 3 weeks ago by bhutz

  • Status changed from needs_review to positive_review

comment:42 Changed 2 weeks ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict...

comment:43 Changed 2 weeks ago by bhutz

  • Branch changed from u/tscrim/arith_dyn-23497 to u/bhutz/arith_dyn-23497

comment:44 Changed 2 weeks ago by bhutz

  • Commit changed from bdbf658dcbb2005227228a13330d5d4090d33724 to 2eecc68b05527c59db06daba1bedadc5756e8e05
  • Status changed from needs_work to needs_review

fixed conflict


New commits:

2eecc68Merge branch 8.1.beta5 into t/23497/arith_dyn-23497

comment:45 Changed 2 weeks ago by tscrim

  • Status changed from needs_review to positive_review

Hopefully this will get high merge priority...

comment:46 Changed 11 days ago by vbraun

  • Branch changed from u/bhutz/arith_dyn-23497 to 2eecc68b05527c59db06daba1bedadc5756e8e05
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.