-
-
Notifications
You must be signed in to change notification settings - Fork 491
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement infinite sums and products for lazy series #35362
Implement infinite sums and products for lazy series #35362
Conversation
@mantepse Here is a sketch for infinite products using lazy series based upon our conversion about this from many months ago. There are likely many ways it could be improved, but I wanted you to take a look at it before I started to polish it and copy to infinite sums. In principle, I could get rid of the Anyways, I am off to bed. If you get a chance today, let me know what you think. |
I will look properly tomorrow - maybe you could meanwhile merge develop? One thing: essentially you only use Also, it would be good to include a check that it interacts nicely with recursive definitions. Spoiler: looks good.
In the docstring of Did you check whether padics has similar functionality? One thing that I do not really like is the dependency of the semantics on the input:
Wouldn't it be better to separate the two functionalities? I don't know whether there is an established name for the infinite product of this specific form, though. |
Possibly Besides, here is possibly a bug:
does not return. |
I thought a lot about how to name it, and I couldn't come up with something natural beyond Indeed, that seems like a bug. It does "return" but it seems to have trouble getting coefficients. It probably has to do with not taking in the minimal valuation. This likely should be passed to the stream (which would be the default for |
Also, I did not check if padics has similar functionality. I agree with you about passing |
Another (different) approach: wouldn't it be equally good to require that the factors passed are of the form I guess that your approach is slightly faster, but then we could have two methods, |
I thought about this, but I quickly came to the conclusion it is not a good idea. We would have to either manually implement something to check the next nonzero coefficient (and essentially duplicate our
I also believe it to be faster. It is also better in terms of memory as the caches don't store all of those intermediate TL;DR We can make it more natural by having |
Perhaps another option is we assume the user is giving good input (which we very clearly must warn them about). It would have an advantage of avoiding a trivial |
Perhaps it actually isn't so bad; I forgot that I did a custom check for the valuation on the components since I didn't need to compute their actual order. |
I could not find anything in padics. I would still prefer to have a separate method for products of the form I agree that accepting the factors as input (i.e., including the 1) doesn't look very attractive, either. |
I strongly think that all infinite products should be under |
If it must be, and you make Currently sage doesn't seem to have any infinite product at all. |
088aa6e
to
e6a0ba5
Compare
I changed it to have the iterator output To get rid of the infinite loop trap, I can change it so the order is only increased one step at each time. Although for something more "sparse" in regards to the orders of Anyways, that's all for me tonight; I need some sleep. |
If we are mostly okay with the general format of the infinite product, I will also add the infinite sums now too. |
e6a0ba5
to
d709c1e
Compare
d709c1e
to
5514c89
Compare
I added the doctests and fixed a few small things (it handles trivial zeros in the iterator). This is now ready for review. |
Closes #34404. |
I discovered a problem, which I don't understand yet:
works, but
raises a |
That is a good example. It shows that my implementation was computing coefficients when it shouldn't have been. I have changed things around so that it computes fewer coefficients unless it absolutely needs to. The drawback is that we need to be a bit more careful with having the approximate order weakly increasing when using undefined series in infinite sums/products. |
Documentation preview for this PR (built with commit 2afd7f3; changes) is ready! 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Thank you. |
<!-- Please provide a concise, informative and self-explanatory title. --> <!-- Don't put issue numbers in the title. Put it in the Description below. --> <!-- For example, instead of "Fixes sagemath#12345", use "Add a new method to multiply two integers" --> We allow lazy series to compute infinite sums `\sum_{i \in I} p_i` and products `\prod_{i \in I} (1 + p_i)` over arbitrary (enumerated) index sets `I` subject to a somewhat mild technical constraint that the order of each input `p_i` is weakly increasing wrt the iteration order and the preimage of each order is finite. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x ]`. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> - sagemath#35265 <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#35362 Reported by: Travis Scrimshaw Reviewer(s): Martin Rubey, Travis Scrimshaw
We allow lazy series to compute infinite sums
\sum_{i \in I} p_i
and products\prod_{i \in I} (1 + p_i)
over arbitrary (enumerated) index setsI
subject to a somewhat mild technical constraint that the order of each inputp_i
is weakly increasing wrt the iteration order and the preimage of each order is finite.📝 Checklist
⌛ Dependencies