Skip to content
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

implement derivatives of lazy series #34413

Closed
mantepse opened this issue Aug 23, 2022 · 43 comments
Closed

implement derivatives of lazy series #34413

mantepse opened this issue Aug 23, 2022 · 43 comments
Labels
c: combinatorics p: major / 3 t: enhancement
Milestone
sage-9.8

Comments

@mantepse
Copy link
Contributor

We implement derivatives for LazyLaurentSeries, LazyTaylorSeries and LazySymmetricFunctions

Depends on #34383

CC: @tscrim

Component: combinatorics

Keywords: LazyPowerSeries

Author: Martin Rubey

Branch/Commit: cdea820

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/34413
@mantepse mantepse added this to the sage-9.7 milestone Aug 23, 2022
@mantepse
Copy link
Contributor Author

Branch: u/mantepse/implement_derivatives_of_lazy_series

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 23, 2022

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

55761b8better derivative for LazyLaurentSeries

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 23, 2022

Commit: 55761b8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 23, 2022

Changed commit from 55761b8 to 50ce450

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 23, 2022

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

50ce450derivative for LazyTaylorSeries

@mantepse
Copy link
Contributor Author

Changed keywords from none to LazyPowerSeries

@mantepse

This comment has been minimized.

Sign in to view
@mantepse
Copy link
Contributor Author

Author: Martin Rubey

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 23, 2022

Changed commit from 50ce450 to 10cfc11

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 23, 2022

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

10cfc11derivative for LazySymmetricFunctions

@mantepse
Copy link
Contributor Author

Dependencies: #32324

@mantepse
Copy link
Contributor Author

Changed dependencies from #32324 to #34383

@tscrim
Copy link
Collaborator

tscrim commented Aug 25, 2022

comment:9

I don't think there is anything we need to worry about with regards to analytic stuff (since there is an assumption the derivatives commute), but I want to confirm you also think so too.

Also you have empty INPUT: blocks. Either remove, add the inputs, or provide a link to something that explains how to parse the inputs.

@tscrim
Copy link
Collaborator

tscrim commented Aug 25, 2022

Reviewer: Travis Scrimshaw

@mantepse
Copy link
Contributor Author

comment:10

Replying to @tscrim:

I don't think there is anything we need to worry about with regards to analytic stuff (since there is an assumption the derivatives commute), but I want to confirm you also think so too.

That's correct.

Also you have empty INPUT: blocks. Either remove, add the inputs, or provide a link to something that explains how to parse the inputs.

Done.

Besides, I now know of a way to make sense of

sage: L.<x,y> = LazySymmetricFunction(SymmetricFunctions(QQ).s())
sage: f = 1/(1-x*y)

The natural thing is to set x = tensor([s[1], s[0]]) and y = tensor([s[0], s[1]]). Doing so, also

sage: f.derivative(x, 2, y)

makes sense. In fact, this is what we do all the time in the doctests anyway.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 30, 2022

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

07ca029add a consistency check
1d559ccimprove composing Taylor series, start making plethysm more general
c47b060Doing some changes to normal symmetric functions plethysm to support coefficients and lazy symmetric functions.
5776c61Adding the category of commutative rings to the category of tensor products of commutative algebras.
5f21f3dMerge branch 'public/categories/tensor_commutative_rings-34453' into public/rings/lazy_series_revert-34383
a2e9ed7Making the test for coercion of plethysms wrt tesnor products more robust.
94219d4fix plethysm with f having finite support
6745cf1Some last details and optimizations to Stream_plethysm.
3d6d39ffix typo in comment
ee35418Merge branch 'public/rings/lazy_series_revert-34383' of trac.sagemath.org:sage into t/34413/implement_derivatives_of_lazy_series

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 30, 2022

Changed commit from 10cfc11 to ee35418

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 30, 2022

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

dafdcb4fix doctests
9124987Merge branch 'public/rings/lazy_series_revert-34383' of trac.sagemath.org:sage into t/34413/implement_derivatives_of_lazy_series

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 30, 2022

Changed commit from ee35418 to 9124987

@tscrim
Copy link
Collaborator

tscrim commented Aug 30, 2022

comment:13

Replying to @mantepse:

Replying to @tscrim:
Besides, I now know of a way to make sense of

sage: L.<x,y> = LazySymmetricFunction(SymmetricFunctions(QQ).s())
sage: f = 1/(1-x*y)

The natural thing is to set x = tensor([s[1], s[0]]) and y = tensor([s[0], s[1]]). Doing so, also

sage: f.derivative(x, 2, y)

makes sense. In fact, this is what we do all the time in the doctests anyway.

I agree that it makes a little bit of sense to think of the variable as a the unique degree one element (up to scalar). However, this would be a hack IMO because the degree one elements do not generate the ring of symmetric functions (as it is a freely generated by ei). So the result of gens() would be wrong if it was the degree one elements. Now a way to explain yourself out of this is that the R.<x,y> type syntax returns the first (in some ordering) n generators.

I still do not think this extra little agreement with a completely different class (where I expect no such compatibility) and implementation is worth all of this extra complexity and effort. Although I won’t oppose it because you are not going to create some extra class for the variables (at least, judging by what you’re saying). However, to me it adds to maintenance costs without a clear benefit.

Actually, isn’t what is happening just a renaming of “skew_by“? Will that be implemented here too?

@mantepse
Copy link
Contributor Author

comment:14

you never sleep, do you?

Yes, this is skey_by(p[1]).

Indeed, it is a bit unclear to me whether it makes sense to implement all the methods on SymmetricFunctions also on LazySymmetricFunctions. It would be nicer to have an automatism for those which are graded maps.

However, that's almost certainly for a different project.

@tscrim
Copy link
Collaborator

tscrim commented Aug 31, 2022

comment:15

Replying to @mantepse:

you never sleep, do you?

I wanted to convey my thoughts while you were still awake in case you wanted to respond.

Yes, this is skey_by(p[1]).

It seems like that is your mathematical model is disguise. It just happens to have all of the other usual derivative properties when doing p[1].

Indeed, it is a bit unclear to me whether it makes sense to implement all the methods on SymmetricFunctions also on LazySymmetricFunctions. It would be nicer to have an automatism for those which are graded maps.

However, that's almost certainly for a different project.

I think we can port them over as we find them to be useful.

Anyways, for follow-up tickets.

Can you add the following tests:

(For exact series with nonzero constant)

sage: L.<z> = LazyLaurentSeriesRing(ZZ)
sage: f = L([1,2], valuation=-2, constant=1)
sage: f
z^-2 + 2*z^-1 + 1 + z + z^2 + O(z^3)
sage: f.derivative()
-2*z^-3 - 2*z^-2 + 1 + 2*z + 3*z^2 + 4*z^3 + O(z^4)

Calculus rules (up to an implicit sign choice):

sage: L.<z> = LazyLaurentSeriesRing(QQ)
sage: log(1-z).derivative()
-1 - z - z^2 - z^3 - z^4 - z^5 - z^6 + O(z^7)

Side note: This doesn't work over ZZ because we are too greedy about converting things to the ring. Indeed, the failure for ZZ stems from

sage: L.<z> = LazyLaurentSeriesRing(ZZ)
sage: log(1-z)
<repr(<sage.rings.lazy_series_ring.LazyLaurentSeriesRing_with_category.element_class at 0x7f2c90572c80>) failed: TypeError: no conversion of this rational to integer>

This is the only stream that is forcing the output to be in a particular ring and an explicit example about why we shouldn't do that in Streams. We should change this on a separate ticket (or here if you want).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 31, 2022

Changed commit from 9124987 to 610078f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 31, 2022

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

610078fadd doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 31, 2022

Changed commit from 610078f to 5666eae

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2022

Changed commit from 5666eae to none

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2022

Changed dependencies from #34383 to #34383, #34494

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2022

comment:21

The doctest fails due to changes in #34494. Trivial doctest update.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 22, 2022

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. Last 10 new commits:

ee35418Merge branch 'public/rings/lazy_series_revert-34383' of trac.sagemath.org:sage into t/34413/implement_derivatives_of_lazy_series
dafdcb4fix doctests
9124987Merge branch 'public/rings/lazy_series_revert-34383' of trac.sagemath.org:sage into t/34413/implement_derivatives_of_lazy_series
610078fadd doctests
731072aremove unnecessary code leftover from merge, and remove unneccesary import detected by pyflakes
5666eaeMerge branch 'public/rings/lazy_series_revert-34383' of trac.sagemath.org:sage into t/34413/implement_derivatives_of_lazy_series
1729827Merge branch 'u/mantepse/implement_derivatives_of_lazy_series' of https://github.com/sagemath/sagetrac-mirror into u/tscrim/derivatives_lazy_series-34413
ab98f2eless verbose monomials in shuffle algebras
5d76825Merge branch 'u/chapoton/34494' of https://github.com/sagemath/sagetrac-mirror into u/tscrim/derivatives_lazy_series-34413
97df300Updating sf/sfa.py doctest due to #34494.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 22, 2022

Commit: 97df300

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2022

comment:23

Slightly too fast with updating the ticket.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 22, 2022

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. Last 10 new commits:

fbccb39normalize degree in Stream_exact for correct equality
b930f58Merge branch 'u/mantepse/categories_of_lazy_series' of https://github.com/sagemath/sagetrac-mirror into u/tscrim/categories_lazy_series-34470
b47407bImplementing `_floordiv_`, Stream._true_order to boolean, first fraction field, more tests, marking long tests.
e077b66Using dynamic classes for FPS gcd.
ddc04a1Fixing last pyflakes things.
9260d14Merge branch 'u/tscrim/categories_lazy_series-34470' of https://github.com/sagemath/sagetrac-mirror into u/tscrim/derivatives_lazy_series-34413
1388b8aMerge branch 'u/chapoton/34494' of https://github.com/sagemath/sagetrac-mirror into public/rings/lazy_series_revert-34383
7bfe5f7Merge branch 'public/rings/lazy_series_revert-34383' of https://github.com/sagemath/sagetrac-mirror into public/rings/lazy_series_revert-34383
2039990Updating doctest due to changes from #34494.
1b9897dMerge branch 'public/rings/lazy_series_revert-34383' into u/tscrim/derivatives_lazy_series-34413

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 22, 2022

Changed commit from 97df300 to 1b9897d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 22, 2022

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

cdea820Merge branch 'public/rings/lazy_series_revert-34383' of https://github.com/sagemath/sagetrac-mirror into u/tscrim/derivatives_lazy_series-34413

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 22, 2022

Changed commit from 1b9897d to cdea820

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2022

Changed dependencies from #34383, #34494 to #34383

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2022

comment:26

Trivial update of #34383, which should have had the #34494 change instead.

@mantepse
Copy link
Contributor Author

Changed branch from u/tscrim/derivatives_lazy_series-34413 to u/mantepse/derivatives_lazy_series-34413

@vbraun
Copy link
Member

vbraun commented Sep 27, 2022

Changed branch from u/mantepse/derivatives_lazy_series-34413 to cdea820

@vbraun vbraun closed this as completed in ad97e1b Sep 27, 2022
This was referenced Sep 29, 2022
Closed
Closed
@tornaria tornaria mentioned this issue Feb 14, 2023
Merged
2 tasks
vbraun pushed a commit that referenced this issue Mar 26, 2023
gh-35127: Fix a very slow doctest in sage/data_structures/stream.py
53362d1 
    
This single test used to take ~ 200s.
Now it takes ~ 1.5s and I marked it long time.

This was introduced in 94219d4 (#34413).

<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes #1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description

<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes #1337" -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [x] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [x] I have linked an issue or discussion.

### ⌛ Dependencies
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
    
URL: #35127
Reported by: Gonzalo Tornaría
Reviewer(s): Gonzalo Tornaría, Martin Rubey, Travis Scrimshaw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: combinatorics p: major / 3 t: enhancement
Projects
None yet
Milestone
sage-9.8
Development

No branches or pull requests
4 participants
@vbraun @tscrim @mkoeppe @mantepse