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

Evolve or nix broadcast_zpreserving[!]? #19533

Closed
Sacha0 opened this issue Dec 8, 2016 · 4 comments
Closed

Evolve or nix broadcast_zpreserving[!]? #19533

Sacha0 opened this issue Dec 8, 2016 · 4 comments
Labels
design Design of APIs or of the language itself needs decision A decision on this change is needed sparse Sparse arrays

Comments

@Sacha0
Copy link
Member

Sacha0 commented Dec 8, 2016

broadcast_zpreserving[!] broadcasts a function over the intersection of the nonzero patterns of one SparseMatrixCSC and either another SparseMatrixCSC or an Array, BitArray, or Number. This issue's purpose is to determine whether broadcast_zpreserving[!] should (1) evolve, (2) disappear, or (3) be left in peace.

On the one hand, broadcast_zpreserving[!] sees little use in its present (unexported) form. Chances are removing it would cause little disruption and few would miss it. One less function to support and maintain, more time for higher priority work.

On the other hand, some desire exists for related map-/broadcast-like functionality for sparse/structured matrices/vectors (see discussion in, e.g., #7010, #11474, and #18590). A generalization of broadcast_zpreserving[!] could satisfy most such desire. And with map/broadcast's recent evolution for sparse/structured matrices (where, e.g., returning a SparseMatrixCSC from map/broadcast operations over structured matrices seems the present consensus direction), a generalization of broadcast_zpreserving[!] could play a more significant role going forward: That generalization could strictly preserve structure, e.g. returning Diagonal when operating over a Diagonal and a Tridiagonal (or Matrix, or what have you).

Thoughts? Thanks and best!

@Sacha0 Sacha0 added needs decision A decision on this change is needed design Design of APIs or of the language itself sparse Sparse arrays labels Dec 8, 2016
@Sacha0 Sacha0 changed the title Evolve or nix broadcast_zpreserving? Evolve or nix broadcast_zpreserving[!]? Dec 8, 2016
@nalimilan
Copy link
Member

How do this relate to your PRs in this area, e.g. #19518?

@Sacha0
Copy link
Member Author

Sacha0 commented Dec 10, 2016

How do this relate to your PRs in this area, e.g. #19518?

On the one hand, those pull requests largely render broadcast_zpreserving[!]'s present incarnation obsolete: broadcast[!] now provides (more generally, if less efficiently in some cases) the majority of broadcast_zpreserving[!]'s existing functionality. Hence this issue.

On the other hand, carrying that series of pull requests through to its logical conclusion may make the demand for broadcast_zpreserving[!]-related functionality more acute.

In any case, deciding what to do with broadcast_zpreserving[!] should not prevent that line of work from moving forward.

Does that clarify?

My immediate, malleable inclination is to remove broadcast_zpreserving[!] (its hypothetical evolved form would look completely different anyway), close this issue, carry the present sparse/structured map[!]/broadcast[!] work through to its logical conclusion, assess the impact of those changes in practice over the next few months, and thereafter consider whether the potential value of an evolved broadcast_zpreserving[!] warrants its cost and prioritization.

Thoughts? Thanks and best!

@stevengj
Copy link
Member

+1 for removing it. Zero-preserving functions (assumed pure) should be detected automatically by broadcast.

@Sacha0
Copy link
Member Author

Sacha0 commented Feb 22, 2017

Closing given #19720 and generic sparse broadcast[!].

Closing thought: Generic sparse broadcast[!] operates over the union of the patterns of its arguments, and necessarily returns either a SparseVector or SparseMatrixCSC for type stability (given that the result must be able to contain a nonzero in any location). A function that operates only over the intersection of the patterns of its arguments and yields zeros outside that intersection (sort of the opposite extreme from generic sparse broadcast[!], modeling operations like *) might prove a useful complement, providing most of the functionality discussed elsewhere that generic sparse broadcast[!] does not provide. (Notably such a function could type-stably produce e.g. a Diagonal from operations over combinations of Diagonals or Diagonals and broader-pattern types.) Best!

@Sacha0 Sacha0 closed this as completed Feb 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design of APIs or of the language itself needs decision A decision on this change is needed sparse Sparse arrays
Projects
None yet
Development

No branches or pull requests

3 participants