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.

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

enhancements in infinite polynomial rings #36651

Merged
merged 1 commit into from
Jan 14, 2024

Conversation

fchapoton
Copy link
Contributor

@fchapoton fchapoton commented Nov 5, 2023

This make a few little changes, nothing serious.

This catches more precise exceptions, not Exception.

@mantepse
Copy link
Collaborator

mantepse commented Nov 5, 2023

Thank you! It appears to me that the all_gens method is completely new (in Sage), do you have any plans with it?

@fchapoton
Copy link
Contributor Author

well, not really. The problem is that the usual semantic of gen and gens is not respected in this file. They do not return some elements of the ring. I am not sure what to do. I can remove all_gens or call it otherwise or make it hidden..

@mantepse
Copy link
Collaborator

mantepse commented Nov 5, 2023

Do you know whether there is any good reason for not respecting the usual semantics?

@fchapoton
Copy link
Contributor Author

it's because there are infinitely many generators, in a finite number of families

@mantepse
Copy link
Collaborator

mantepse commented Nov 5, 2023

I don't see how that's a good reason. Is this the only infinite dimensional ring in sage which has gen and gens?

@fchapoton
Copy link
Contributor Author

It is also a matter of offering a simple and practical access to the variables.

@mantepse
Copy link
Collaborator

mantepse commented Nov 6, 2023

Can we have both, in case it makes sense? I find

sage: P.<a, b> = InfinitePolynomialRing(QQ)

practical, and I do not see why we couldn't have, at the same time

sage: [P.gen(i) for i in range(5)]
[a[0], b[0], a[1], b[1], a[2]]

I looked at the code: on the plus side, ParentWithGens is deprecated, on the minus side, there is no method returning a family of generators.

I think that there are quite a few things that would work better with gen returning a proper generator of the ring, for examples substitution and defining homomorphisms.

@mantepse
Copy link
Collaborator

mantepse commented Nov 6, 2023

@fchapoton
Copy link
Contributor Author

See also #34120

@fchapoton
Copy link
Contributor Author

I do not want to do big things and changes here. So maybe we can keep only the changes that enlarge the possible input of the gen method of infinite polynomial rings ? Would you approve that ?

@mantepse
Copy link
Collaborator

I would rather not modify the semantics before agreeing on the desirable semantics, especially since we seem to agree that the current semantics breaks stuff quite badly. Do you have any concrete plans with your proposed improvement?

@fchapoton
Copy link
Contributor Author

I dot not have any specific plan about gen and gens. I may try to get rid of ParentWithGens when I have time.

So we can close the present poule-requete, vielleicht ?

@fchapoton
Copy link
Contributor Author

Ok, I have replaced the branch by a new one only making very simple changes, and not touching anything about gens

Copy link

Documentation preview for this PR (built with commit 96d8a84; changes) is ready! 🎉

@mantepse
Copy link
Collaborator

Can I assume that the failure in singular is unrelated?

File "src/sage/interfaces/singular.py", line 530, in sage.interfaces.singular.Singular._send_interrupt
Failed example:
    2*a
Expected:
    2
Got:
    <BLANKLINE>

If so, positive review from my side.

@fchapoton
Copy link
Contributor Author

Well, it's only on conda+mac, and not on the main Build and Test or in other conda builds. So, yes, unrelated.
Please approve and set to positive. Thanks for your review !

Copy link
Collaborator

@mantepse mantepse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@vbraun vbraun merged commit 9ec9de0 into sagemath:develop Jan 14, 2024
19 of 21 checks passed
@mkoeppe mkoeppe added this to the sage-10.3 milestone Jan 14, 2024
@fchapoton fchapoton deleted the infinite_poly_gen branch January 15, 2024 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants