8000 Adding the polylogarithm as lazy series by tscrim · Pull Request #40141 · sagemath/sage · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Adding the polylogarithm as lazy series #40141

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 14, 2025
Merged

Adding the polylogarithm as lazy series #40141

merged 4 commits into from
Jun 14, 2025

Conversation

tscrim
Copy link
Collaborator
@tscrim tscrim commented May 21, 2025 •
edited
Loading

Another special function that is important in many areas of math.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@tscrim
Copy link
Collaborator Author
tscrim commented May 21, 2025

@mantepse Another special function example. I might need to rename the method to dilog and polylog so the corresponding functions call these methods. I will check later.

@grhkm21
Copy link
Contributor
grhkm21 commented May 21, 2025

Would it be a good idea to have s=2 by default in polylog? Then dilog is just an alias

@tscrim
Copy link
Collaborator Author
tscrim commented May 21, 2025

No, then it is not an alias. 2 also isn't a particularly special value for the $s$ input.

Separately, calling the method dilog works for the function redirecting. But not for polylog, probably due to the extra input?

@tscrim tscrim force-pushed the lazy_series/polylog branch from f6100ff to d23761b Compare May 22, 2025 08:36
@mantepse
Copy link
Contributor

@mantepse Another special function example. I might need to rename the method to dilog and polylog so the corresponding functions call these methods. I will check later.

I think it would be good to be consistent with the naming in any case.

Currently, I get:

sage: L.<x> = LazyLaurentSeriesRing(SR)
sage: x.dilogarithm()
1.0*x + 0.25*x^2 + 0.1111111111111111*x^3 + 0.0625*x^4 + 0.04*x^5 + 0.027777777777777776*x^6 + 0.02040816326530612*x^7 + O(x^8)

which I find surprising.

@mantepse
Copy link
Contributor

I would rather remove polylogarithm and dilogarithm completely, or is there a really good reason to have them?

@tscrim
Copy link
Collaborator Author
tscrim commented May 22, 2025

The issue with the SR output is coming from the fact it is first doing stuff as Python int's:

sage: int(2) ** int(-2)
0.25

I've fixed this by converting n to the base ring first..

I think it is good to have a verbose version. It doesn't really hurt the auto-completion. However, this is not a strong opinion, so you still want to remove the long form?

@tscrim tscrim force-pushed the lazy_series/polylog branch 2 times, most recently from ddd352d to 59b1913 Compare May 22, 2025 08:56
@mantepse
Copy link
Contributor

I would either remove the long form or add the long form in the other cases, although I tend to removal.

I know that it's only me, but having many completions in the emacs buffer (I don't know other environments) is a nuisance. I need to check whether the methods are really the same when I am looking for something.

(I also dislike aliases when debugging - but this doesn't really apply here). I find the definitions of a method by doing grep "def fun", which of course fails for aliases (and also for cdef functions). I don't know of a better method.)

@tscrim
Copy link
Collaborator Author
tscrim commented May 22, 2025

Okay, I have changed it to only use the short form. I guess this is consistent with the other special functions (e.g., log) too. I added a few other examples of identities I found online.

@mantepse
Copy link
Contributor

The issue with the SR output is coming from the fact it is first doing stuff as Python int's:

sage: int(2) ** int(-2)
0.25

I've fixed this by converting n to the base ring first..

Possibly (but certainly not in this PR) we should warn in the doc of _element_constructor_ that a callable will be evaluated at python int's. The alternative would be to convert n to an Integer in the _element_constructor_. Currently, we do things like

                    coeff_stream = Stream_function(lambda i: BR(x(i)), self._sparse, valuation)

Not sure. Doing things twice is not good either.

@mantepse
Copy link
Contributor

#40093 has extra whitespace, apart from that this looks great!

@tscrim
Copy link
Collaborator Author
tscrim commented May 24, 2025

Adding a warning about this is definitely good. Although it isn’t doing anything twice and passing an Integer means it is less likely an int leaks out. That is also a very fast conversion. So I might lean to making sure the input is a Sage Integer. However, that is indeed for a separate PR.

Since the whitespace is on the dependency, it is taken care of on that PR. Hence, you can set this to a positive review.

@tscrim tscrim force-pushed the lazy_series/polylog branch from d947e6f to ea8d580 Compare June 9, 2025 00:01
@github-actions GitHub Actions
Copy link
github-actions bot commented Jun 9, 2025

Documentation preview for this PR (built with commit ea8d580; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 9, 2025
2b2c35a 
sagemathgh-40141: Adding the polylogarithm as lazy series
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Another special function that is important in many areas of math.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->

- sagemath#40093: Adds to the same places in the files.
    
URL: sagemath#40141
Reported by: Travis Scrimshaw
Reviewer(s):
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 9, 2025
cd4bd29 
sagemathgh-40141: Adding the polylogarithm as lazy series
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Another special function that is important in many areas of math.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->

- sagemath#40093: Adds to the same places in the files.
    
URL: sagemath#40141
Reported by: Travis Scrimshaw
Reviewer(s):
@vbraun vbraun merged commit bcdc712 into sagemath:develop Jun 14, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
c: algebra t: enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tscrim @grhkm21 @mantepse @vbraun
0