Opened 4 years ago
Closed 4 years ago
#23497 closed enhancement (fixed)
relocate dynamical systems code from sage/schemes to sage/dynamics
Reported by:  bhutz  Owned by:  

Priority:  major  Milestone:  sage8.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, GitHub, GitLab)  Commit:  2eecc68b05527c59db06daba1bedadc5756e8e05 
Dependencies:  Stopgaps: 
Description (last modified by )
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 4 years ago by
 Branch set to u/bhutz/arith_dyn
 Commit set to b4d09658f577eb9be7d232e5ef25a310c330c028
comment:2 Changed 4 years ago by
 Branch changed from u/bhutz/arith_dyn to u/xander.faber/arith_dyn
comment:3 Changed 4 years ago by
 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:
0a40209  Fixed a docstring in morphism.py

f6827d0  AffineSpace now accepts a univariate polynomial ring and constructs A^1

586f17f  Improved dynamical system constructors in generic_ds module

4950fcc  Streamlined dynamical system class init's after pushing hard work to the constructors in generic_ds module

comment:4 Changed 4 years ago by
 Commit changed from 4950fcc87670d8a7625b31627e3ceae1cca86b48 to b9327f83f69e74707e3dc597ba19ec4f9b79b57d
comment:5 Changed 4 years ago by
 Branch changed from u/xander.faber/arith_dyn to u/bhutz/arith_dyn
comment:6 Changed 4 years ago by
 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 positivereivew even if it is ready.
New commits:
f9f291c  23497: reinstate doctests for deprecated functions

ae729ce  23497: tests, docs, and init updates to generic_ds.py

296dccd  23497: finish other dynamics files and all docs

comment:7 Changed 4 years ago by
 Commit changed from 296dccdc9efe4c8b48e69cb10c4799bd94e6fa9e to 2c604d062974dfb5241b8027ca06c0ce5c0cd2fe
Branch pushed to git repo; I updated commit sha1. New commits:
2c604d0  Merge branch 8.1.beta0 into t/23497/arith_dyn

comment:8 Changed 4 years ago by
Build succeeds, as do all doctests. I walked through the documentation and some examples of the new constructors ... looks pretty solid.
comment:9 Changed 4 years ago by
The branch is red above, meaning that it does not apply on the latest beta (8.1.b1).
comment:10 Changed 4 years ago by
Yes, I'm aware. This probably also conflicts with a number of tickets currently positivereview. I plan to do all that merging today now that the functionality for this ticket is stable.
comment:11 Changed 4 years ago by
 Commit changed from 2c604d062974dfb5241b8027ca06c0ce5c0cd2fe to ba22abdc1b2d85f716c0512fcb7b0ec86fb8dbf1
Branch pushed to git repo; I updated commit sha1. New commits:
ba22abd  Merge branch 8.1.beta1 into t/23497/arith_dyn

comment:12 Changed 4 years ago by
 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:14 Changed 4 years ago by
 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 4 years ago by
great, thanks. I was going to do this today as soon as I finish the rebase for a different ticket.
comment:16 Changed 4 years ago by
 Commit changed from ba22abdc1b2d85f716c0512fcb7b0ec86fb8dbf1 to 79378c30d23f4c3da284272115634e3776ef8c81
Branch pushed to git repo; I updated commit sha1. New commits:
79378c3  Merge branch 8.1.beta3 into t/23497/arith_dyn

comment:17 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:18 Changed 4 years ago by
 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 oneline 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 permainstance 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 4 years ago by
 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 cleanup 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 4 years ago by
 Commit changed from 79378c30d23f4c3da284272115634e3776ef8c81 to 80ff2e673f93c961618923251d8d3d19786ad015
Branch pushed to git repo; I updated commit sha1. New commits:
80ff2e6  23497: some fixes from review

comment:21 followup: ↓ 23 Changed 4 years ago by
 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:
80ff2e6  23497: some fixes from review

comment:22 Changed 4 years ago by
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 4 years ago by
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 followup: ↓ 25 Changed 4 years ago by
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 cleanup.) 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 4 years ago by
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 cleanup.) 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 4 years ago by
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 4 years ago by
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 4 years ago by
I can do this tomorrow morning when I wake up if you want.
comment:29 Changed 4 years ago by
 Commit changed from 80ff2e673f93c961618923251d8d3d19786ad015 to cc50108eb32d1fc33d030cc128eecbf6747d67a9
comment:30 Changed 4 years ago by
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 cleanup 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 4 years ago by
 Branch changed from u/bhutz/arith_dyn to u/tscrim/arith_dyn23497
 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 microoptimizations I know.
If my changes look good, then positive review.
New commits:
4bbdcde  Fixing things with __classcall_private__.

7408d79  Cleaning up the doc and improving the code.

comment:32 Changed 4 years ago by
 Branch changed from u/tscrim/arith_dyn23497 to u/bhutz/arith_dyn23497
comment:33 Changed 4 years ago by
 Commit changed from 7408d79a2c8833d0c496cc32a3755f1405bc2151 to d00cea258b3b5797ca06ddd33ac55bf4e6ae701c
 Status changed from needs_review to positive_review
comment:34 Changed 4 years ago by
No problem. Thanks for making the initial round of fixes and adapting the code.
comment:36 Changed 4 years ago by
 Commit changed from d00cea258b3b5797ca06ddd33ac55bf4e6ae701c to 300a3b1fe5773b7983736cbbed23ed13c15e20b7
Branch pushed to git repo; I updated commit sha1. New commits:
300a3b1  Merge branch 8.1.beta4 into t/23497/arith_dyn23497

comment:38 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:39 Changed 4 years ago by
 Status changed from positive_review to needs_work
doctest_warning
> doctest:warning
sage t long /usr/lib64/python2.7/sitepackages/sage/schemes/projective/projective_morphism.py ********************************************************************** File "/usr/lib64/python2.7/sitepackages/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/pythonexec/python2.7/sageruntests", line 114, in <module> err = DC.run() File "/usr/lib64/python2.7/sitepackages/sage/doctest/control.py", line 1099, in run self.run_doctests() File "/usr/lib64/python2.7/sitepackages/sage/doctest/control.py", line 824, in run_doctests self.dispatcher.dispatch() File "/usr/lib64/python2.7/sitepackages/sage/doctest/forker.py", line 1777, in dispatch self.parallel_dispatch() File "/usr/lib64/python2.7/sitepackages/sage/doctest/forker.py", line 1667, in parallel_dispatch w.start() # This might take some time File "/usr/lib64/python2.7/sitepackages/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/sitepackages/sage/doctest/forker.py", line 1916, in run task(self.options, self.outtmpfile, msgpipe, self.result_queue) File "/usr/lib64/python2.7/sitepackages/sage/doctest/forker.py", line 2218, in __call__ result = runner.run(test) File "/usr/lib64/python2.7/sitepackages/sage/doctest/forker.py", line 663, in run return self._run(test, compileflags, out) File "/usr/lib64/python2.7/sitepackages/sage/doctest/forker.py", line 518, in _run self.compile_and_execute(example, compiler, test.globs) File "/usr/lib64/python2.7/sitepackages/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/sitepackages/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/sitepackages/sage/misc/superseded.py", line 101, in deprecation warning(trac_number, message, DeprecationWarning, stacklevel) File "/usr/lib64/python2.7/sitepackages/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 4 years ago by
 Branch changed from u/bhutz/arith_dyn23497 to u/tscrim/arith_dyn23497
 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:
bdbf658  Fixing typo in doctest.

comment:41 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:42 Changed 4 years ago by
 Status changed from positive_review to needs_work
Merge conflict...
comment:43 Changed 4 years ago by
 Branch changed from u/tscrim/arith_dyn23497 to u/bhutz/arith_dyn23497
comment:44 Changed 4 years ago by
 Commit changed from bdbf658dcbb2005227228a13330d5d4090d33724 to 2eecc68b05527c59db06daba1bedadc5756e8e05
 Status changed from needs_work to needs_review
comment:45 Changed 4 years ago by
 Status changed from needs_review to positive_review
Hopefully this will get high merge priority...
comment:46 Changed 4 years ago by
 Branch changed from u/bhutz/arith_dyn23497 to 2eecc68b05527c59db06daba1bedadc5756e8e05
 Resolution set to fixed
 Status changed from positive_review to closed
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 cleanup to be done. A few things I haven't done yet
New commits:
23497: relocate arithmetic dynamics code: initial version