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

How to handle TPS/ComplexTPS copy constructors #91

Closed
mattsignorelli opened this issue Feb 19, 2024 · 6 comments
Closed

How to handle TPS/ComplexTPS copy constructors #91

mattsignorelli opened this issue Feb 19, 2024 · 6 comments

Comments

@mattsignorelli
Copy link
Contributor

mattsignorelli commented Feb 19, 2024

Currently, there is a bit of an asymmetry between TPS constructors when copying an existing TPS and promoting a number to a TPS. Consider the case:

d1 = Descriptor(1,5)
a = TPS(5)   # has descriptor d1, using GTPSA.desc_current

d2 = Descriptor(2,1)
b = TPS(a)    # Implicitly has descriptor d1
c = TPS(5)    # has descriptor d2, using GTPSA.desc_current

If one would like b to have descriptor d2, they must intentionally write b = TPS(a, use=d2). On one hand, this ensures there is no implicit changing of descriptors, but on the other hand, there is different behavior when the descriptor can be inferred (TPS copy ctor) vs when the descriptor cannot be inferred (promoting a number to a TPS)

@DavidSagan Should we instead make the default behavior be that if use is not explicitly passed, then GTPSA.desc_current is always used regardless of whether or not a number or TPS is passed (and thus there will be some internal implicit descriptor changing)?

@mattsignorelli
Copy link
Contributor Author

I updated my comment to better capture the issue

@DavidSagan
Copy link
Member

We should talk when I get back. There will probably be needed a stack of current descriptors that can be pushed and popped.

@mattsignorelli
Copy link
Contributor Author

Sounds good. There's a lot of nuance here, too. For example, zero(t1) currently uses the same Descriptor as t1, and is used extensively throughout the overloaded operator functions. Making such a change above would make zero(t1) give a Descriptor equal to GTPSA.desc_current. All of the operations would have to be modified to be safe in this sense

@DavidSagan
Copy link
Member

Yes my thought at this point is b = TPS(a) should not change the descriptor.

@mattsignorelli
Copy link
Contributor Author

I think I'd agree, doing anything else might open up a can of worms that's much more complicated than this slightly asymmetric behavior

@mattsignorelli
Copy link
Contributor Author

I'm going to close this, because it seems like the current implementation of letting TPS(t1) be a full copy-ctor including the Descriptor is the best, and it also makes development easier in the normal form package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants