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

Mixed operations between Poly and Number via promote #14

Closed
wants to merge 1 commit into from
Closed

Mixed operations between Poly and Number via promote #14

wants to merge 1 commit into from

Conversation

pwl
Copy link
Collaborator

@pwl pwl commented Nov 23, 2014

I thought that operations mixing Poly and Number could be defined in a simpler way if a number could be promoted to a Poly in a canonical way. I also added a conversion of an array of numbers to an array of polynomials via map.

@StefanKarpinski
Copy link

This may well have a detrimental impact on performance.

@pwl
Copy link
Collaborator Author

pwl commented Nov 23, 2014

I have just tested it and you are indeed correct. My implementation is roughly 100x slower, I didn't notice this earlier. Is there a way to salvage this approach or is it doomed by design?

@StefanKarpinski
Copy link

The issue is that it semantically creates an intermediate polynomial object. The current way that polynomials are represented is not something that the compiler can eliminate, so it has to actually create the object. So, long story short, no, not really.

@pwl
Copy link
Collaborator Author

pwl commented Nov 23, 2014

Ok, I get it. Do you think you could merge the conversion from Number and Array{Number,n} to Poly? I will remove the promotion and related code.

@StefanKarpinski
Copy link

The conversions do seem sensible. I don't have permission to merge stuff on this repo – @Keno can do it.

@pwl pwl closed this Nov 29, 2014
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

Successfully merging this pull request may close these issues.

None yet

2 participants