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

there is a full, its name is widen #24148

Closed
wants to merge 1 commit into from

Conversation

Sacha0
Copy link
Member

@Sacha0 Sacha0 commented Oct 14, 2017

This pull request sketches the second option described in #24147 / provides the functionality discussed in #23270. (After some thought, that generic function has (in an abstract sense) the semantics of widen. So for now this pull request extends widen instead of introducing a new exported name.) Please see #24147 for more. Best!

@Sacha0 Sacha0 added the domain:linear algebra Linear algebra label Oct 14, 2017
@Sacha0 Sacha0 added this to the 1.0 milestone Oct 14, 2017
@Sacha0 Sacha0 mentioned this pull request Oct 14, 2017
@mbauman
Copy link
Sponsor Member

mbauman commented Oct 15, 2017

I don't quite think this name fits.

If x is a type, return a "larger" type (for numeric types, this will be a type with at least as much range and precision as the argument, and usually more). Otherwise x is converted to widen(typeof(x)).

Am I correct in the fact that the defining characteristic of this operation is that we want to ensure that the returned array is completely mutable? Or maybe both dense and completely mutable? If either of those are case, then I think I'd prefer to defer the design here to the broader trait scheme discussion for 1.x. I'm not entirely sure what the answer will be, but it'd be nice to leave the door open here. I think that means not giving this operation a name for now.

@Sacha0
Copy link
Member Author

Sacha0 commented Oct 15, 2017

Am I correct in the fact that the defining characteristic of this operation is that we want to ensure that the returned array is completely mutable? Or maybe both dense and completely mutable? If either of those are case,

Neither of those is the case :). The semantics of this operation are: Given a wrapped array type (the "narrower" type), return the type of the parent of a copy of an instance of that type (the "wider" type). Given a non-wrapped array type, return that array type. Given an instance of an array type, call the operation on the type of that instance and then convert the instance to the resulting type. In approximate terms, strip a wrapping layer.

The name does not matter to me :). (The analogy is quite loose, but the type / instance behavior is roughly the same: widen the type, convert instances to the widened type of that instance.)

I think I'd prefer to defer the design here to the broader trait scheme discussion for 1.x. I'm not entirely sure what the answer will be, but it'd be nice to leave the door open here. I think that means not giving this operation a name for now.

As I express in #24147, I share those sentiments:

Thirteen calls to copy!(similar(parent(A)), A) in a specific context don't alone seem worth exporting a new generic name from Base (nor worth extending an existing Base export in a major way). The generic function in #23270/#24148 may find other uses, but playing with those other uses / carefully vetting behavior prior to introducing such functionality seems wise. And we can introduce such functionality after 1.0, but we can't change or remove it in the 1.x series if we inject it now.

Best!

@JeffBezanson
Copy link
Sponsor Member

I also think widen doesn't really fit. It would kind of make sense for e.g. widen of Bidiagonal to give Tridiagonal, but (1) this is hard to define uniquely, and (2) this isn't what the function does :)

@Sacha0
Copy link
Member Author

Sacha0 commented Oct 15, 2017

Cheers, a different name then if we move forward with these semantics :). The more important question here / in #24147 is whether we need an operation with these or similar semantics at this time (and so far common sentiment seems to favor alternatives in #24147). Best!

@Sacha0 Sacha0 mentioned this pull request Oct 15, 2017
@Sacha0
Copy link
Member Author

Sacha0 commented Oct 19, 2017

Closing in favor of #24147. Thanks all!

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

Successfully merging this pull request may close these issues.

3 participants