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

[WIP] add some junk #24400

Closed
wants to merge 1 commit into from
Closed

[WIP] add some junk #24400

wants to merge 1 commit into from

Conversation

Sacha0
Copy link
Member

@Sacha0 Sacha0 commented Oct 29, 2017

Per triage, base needs more junk. This pull request adds some junk. Specifically, this pull request explores #11557 (comment) / #16029 (comment) by replacing the existing primitive Array constructors with junk equivalents, e.g. Array{T,N}(d::Vararg{Int,N}) -> Array{T,N}(junk, d::Vararg{Int,N}), and reimplementing the existing constructors in terms of those junk equivalents. The next step would be deprecating the existing constructors in favor of those junk equivalents. Best!

@Sacha0 Sacha0 added domain:arrays [a, r, r, a, y, s] status:triage This should be discussed on a triage call labels Oct 29, 2017
@kmsquire
Copy link
Member

kmsquire commented Oct 30, 2017 via email

@Sacha0
Copy link
Member Author

Sacha0 commented Oct 30, 2017

Despite triage liking this, and despite the name being somewhat descriptive of what's actually in the array, I'm not a huge fan of the word junk here. Were any other terms discussed? (I don't actually have any better ideas...)

Alas I do not recall whether and if so what other possibilities were discussed. Descriptiveness was the primary argument in favor of junk IIRC. And possibly precedent.

Having wrung an adequate volume of groanworthy humor from the name, I have no further attachment to it :). Best!

@Sacha0
Copy link
Member Author

Sacha0 commented Oct 30, 2017

Reducing the CI failure to the following test provided a good laugh:

# issue #14878
mutable struct A14878
    ext
end
A14878() = A14878(Dict())
mutable struct B14878
end
B14878(ng) = B14878()
function trigger14878()
    w = A14878()
    w.ext[:14878] = B14878(junk)  # junk not defined!
    return w
end
@test_throws UndefVarError trigger14878()

Someone (quite reasonably) did not anticipate introduction of the name junk to the language 😄.

@JeffBezanson
Copy link
Sponsor Member

junk would be the short and cute choice; the dry and businesslike choice would probably be uninitialized.

@Sacha0
Copy link
Member Author

Sacha0 commented Oct 30, 2017

uninitialized is wonderfully descriptive, if verbose. I recalled another possibility suggested (by Jeff?) during triage: blank. empty might also work, though both blank and empty are less descriptive than uninitialized and junk. Others? Best!

@Sacha0
Copy link
Member Author

Sacha0 commented Nov 2, 2017

Triage would like a better sense of the scope of objection to junk. Thumbs up, thumbs down? :)

(Edit by Stefan:) To clarify: thumbs up is "yeah, I like junk"; thumbs down is "no, I do not like junk".

@martinholters
Copy link
Member

I have no opinion pro or contra junk, but blank and empty sound like there some specific initialization going on, which at least for isbits element types is not the case, so 👎 from me to those two.

@StefanKarpinski
Copy link
Sponsor Member

The current candidates seem to be uninitialized and junk. Here's a pro-junk argument (or an anti-uninitialized argument, depending on how you look at it): we probably want a mode, e.g. for testing, where junk gives you arrays that are actively filled with random values, rather than the array actually being uninitialized; in that case the name uninitialized would actively be wrong, whereas junk remains completely accurate – it's just a question of how much effort you put into generating junk.

Everyone on the triage call is pro-junk or as Jeff said, "I'm not a junk hater". The vote is currently tied 5 to 5. It would be helpful if the junk haters could explain what their objection to the name is.

@KristofferC
Copy link
Sponsor Member

I don't think having a debug possibility of filling uninitialized memory with a known pattern should influence the naming.

@StefanKarpinski
Copy link
Sponsor Member

StefanKarpinski commented Nov 9, 2017

It doesn't influence the naming so much as it highlights the nature of the operation. It doesn't actually mean "give me an uninitialized array" – it means "give me an array full of junk".

@vchuravy
Copy link
Sponsor Member

vchuravy commented Nov 9, 2017 via email

@iamed2
Copy link
Contributor

iamed2 commented Nov 10, 2017

whatever (maybe anything?) sounds more descriptive than junk to me. It took me a minute or two to figure out what was going on from the code. Perhaps because junk is not as common a word, so I assumed it had some special domain-specific meaning here.

At the same time, a comment in each file where junk is used could have cleared that up.

@Sacha0
Copy link
Member Author

Sacha0 commented Nov 10, 2017

I appreciate the argument for anything/whatever, and favor anything between those two. any seems like a reasonable, concise choice as well. (Given the constructor context in which any would appear, collision with the any function might be fine.) That said, junk doesn't imply "random" per se, and so can be understood as anything as well. Best!

@StefanKarpinski
Copy link
Sponsor Member

If we're not going to do junk I think that uninitialized is the best option.

@nalimilan
Copy link
Member

uninitialized sounds good. Maybe also blank or unfilled (as opposed to fill)?

@StefanKarpinski
Copy link
Sponsor Member

Despite the anti-junk contingent taking the lead early on in this nail-biting vote, team junk seems to have pulled ahead with 9 versus 7. So let's go with junk.

@StefanKarpinski StefanKarpinski removed the status:triage This should be discussed on a triage call label Nov 16, 2017
@StefanKarpinski StefanKarpinski added this to the 1.0 milestone Nov 16, 2017
@StefanKarpinski
Copy link
Sponsor Member

@malmaud has made the case that uninitialized has "more dignity" and @JeffBezanson noted that this will be all over our standard library code. The collective feeling is that the anti-junkers have a strong feeling of antipathy towards "junk" while the pro-junkers are at best, mildly amused by the word. So team junk concedes the fight and we can go with uninitialized – for dignity.

@StefanKarpinski
Copy link
Sponsor Member

StefanKarpinski commented Nov 16, 2017

Actual pro-uninitialized argument: a = uninitialized(T, m, n) is pretty much self-explanatory while a = junk(T, m, n) could be a head-scratcher until someone explains to you that it gives you an array with uninitialized memory.

@kmsquire
Copy link
Member

As with most things Julia, I don't always agree with the decisions, but I'm very happy that the language exists and that people are both cordial and sometimes humorous in their responses. I'm happy with the decision here, and also would have been fine if it had gone the other way. Thanks for the discussion!

@rfourquet
Copy link
Member

Also uninitialized seems more descriptive of what is requested (the intent), i.e. not spending resources on initialization (Which will happen manually in a subsequent step) rather than actively wanting junk (the side effect).

@Sacha0
Copy link
Member Author

Sacha0 commented Nov 21, 2017

Closing in favor of #24652. Thanks all!

@Sacha0 Sacha0 closed this Nov 21, 2017
@Sacha0 Sacha0 deleted the morejunk branch November 21, 2017 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:arrays [a, r, r, a, y, s]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants