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

extend sparse map[!]/broadcast[!] to structured matrices #19926

Merged
merged 6 commits into from
Jan 11, 2017

Conversation

Sacha0
Copy link
Member

@Sacha0 Sacha0 commented Jan 8, 2017

(Requires #19724, to which the first four commits belong.) This pull request extends sparse broadcast[!] to structured matrices by promoting structured matrices to sparse where appropriate. (For better future performance, handling structured matrices directly in the underlying sparse broadcast[!] routines might be possible.) Best!

@Sacha0
Copy link
Member Author

Sacha0 commented Jan 8, 2017

@nanosoldier runbenchmarks(ALL, vs = ":master")

@tkelman
Copy link
Contributor

tkelman commented Jan 8, 2017

tests should probably verify the output type? pretty small amount of additional code, that's encouraging.

@Sacha0
Copy link
Member Author

Sacha0 commented Jan 8, 2017

tests should probably verify the output type?

Good call. Output type checks added to the broadcast tests. Thanks!

@Sacha0
Copy link
Member Author

Sacha0 commented Jan 8, 2017

On that note, in the sparse broadcast[!] tests I intended full equality to check both numerical and structural equality, but I now see that sparse full equality only ascertains numerical equality. Have we a stronger sparse equality that checks both numerical and structural equality? If not, perhaps we should add one, at least for testing these routines? Thanks!

@tkelman
Copy link
Contributor

tkelman commented Jan 8, 2017

don't think we have one, but it would make sense to check for testing

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

@kshyatt kshyatt added sparse Sparse arrays broadcast Applying a function over a collection labels Jan 8, 2017
@Sacha0 Sacha0 added this to the 0.6.0 milestone Jan 8, 2017
@Sacha0 Sacha0 changed the title extend sparse broadcast[!] to structured matrices extend sparse map[!]/broadcast[!] to structured matrices Jan 9, 2017
@Sacha0
Copy link
Member Author

Sacha0 commented Jan 9, 2017

Added a commit extending sparse map[!] to structured matrices as well. Best!

@Sacha0
Copy link
Member Author

Sacha0 commented Jan 9, 2017

Rebased out the commits from #19724. Absent objections, requests for time, or someone else merging this PR prior, I plan to merge on Wednesday morning PST. Best!

@Sacha0
Copy link
Member Author

Sacha0 commented Jan 9, 2017

With the additional commit:

@nanosoldier runbenchmarks(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

@Sacha0
Copy link
Member Author

Sacha0 commented Jan 10, 2017

Fixed rebase conflict resolution mistakes. Not certain whether those mistakes could have caused the strange nanosoldier results. (The benchmarks that regressed / improved cover unrelated functionality.) Will rerun nanosoldier if / when Travis and AppVeyor approve. Best!

@Sacha0
Copy link
Member Author

Sacha0 commented Jan 10, 2017

@nanosoldier runbenchmarks(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

@Sacha0 Sacha0 merged commit 29a3563 into JuliaLang:master Jan 11, 2017
@Sacha0 Sacha0 deleted the structuredbc branch January 11, 2017 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broadcast Applying a function over a collection sparse Sparse arrays
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants