Skip to content

Convenience functions for vals#23658

Closed
bramtayl wants to merge 6 commits intoJuliaLang:masterfrom
bramtayl:patch-1
Closed

Convenience functions for vals#23658
bramtayl wants to merge 6 commits intoJuliaLang:masterfrom
bramtayl:patch-1

Conversation

@bramtayl
Copy link
Copy Markdown
Contributor

No description provided.


Val(x) = (@_pure_meta; Val{x}())

eltype(::Val{T}) where T = T
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't correct. T can be Array{S,N}, in which case eltype(Val{Array{S,N}}) should be S. I'd instead make this eltype(::Val{T}) where {T} = eltype(T).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm maybe eltype isn't the right name for what I want. I just want a function that will turn Val{:a}() into :a

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, might be consistent with this:

Julia-0.7.0-DEV> x = Vector{Array{Int,3}}()
0-element Array{Array{Int64,3},1}

Julia-0.7.0-DEV> eltype(x)
Array{Int64,3}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Val isn't a collection or the type of a collection so eltype is not appropriate here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean I could just leave it out this PR, or choose another name? devalue?

@ararslan ararslan added the needs tests Unit tests are required for this change label Sep 10, 2017
eltype(::Val{T}) where T = T

(&)(v1::Val{true}, v2::Val{true}) = Val{true}()
(&)(v1::Val, v2::Val) = Val{false}()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these methods seem to cover too many possibilities; I'd suggest to just write out all four combinations

Copy link
Copy Markdown
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Val shouldn't be used with booleans or places where you want to do computations.

@bramtayl
Copy link
Copy Markdown
Contributor Author

Ive been using value algebra for significant performance gains in places like JuliennedArrays

@bramtayl
Copy link
Copy Markdown
Contributor Author

Val shouldn't be used with booleans or places where you want to do computations.

Why

@vtjnash
Copy link
Copy Markdown
Member

vtjnash commented Sep 11, 2017

See previous attempts at doing this (#22518), the manual (https://docs.julialang.org/en/stable/manual/performance-tips/#Types-with-values-as-parameters-1), the equivalent discussions on lifting Nullablility, and discourse.

@bramtayl
Copy link
Copy Markdown
Contributor Author

Sounds as if there is indeed a specific use case:

This might be worthwhile when the following are true:

> * You require CPU-intensive processing on each Car, and it becomes vastly more efficient if you know the Make and Model at compile time.
> * You have homogenous lists of the same type of Car to process, so that you can store them all in an Array{Car{:Honda,:Accord},N}.

@vtjnash
Copy link
Copy Markdown
Member

vtjnash commented Sep 11, 2017

No, that is saying the opposite: Val is useful for cases where you don't need to do arithmetic on it.

@bramtayl
Copy link
Copy Markdown
Contributor Author

I'm not using this these functions to do arithmetic though...

@bramtayl
Copy link
Copy Markdown
Contributor Author

If by arithmetic you mean repeated functions on different values. Instead, I'm doing the same function on the exact same list of values repeatedly.

@JeffBezanson
Copy link
Copy Markdown
Member

I agree with @vtjnash that these cases should be handled by compiler constant folding instead of manually using Val. In particular ifelse shouldn't be necessary; if you have Val{x} then ifelse(x, ...) should already get constant folded.

@bramtayl
Copy link
Copy Markdown
Contributor Author

It doesn't seem to be working though?

devalue(x::Val{T}) where T = T

Base.ifelse(switch::Val, new, old) = ifelse(devalue(switch), new, old)

tuple_mismatch_error(t...) = error("Mismatched tuples: leftovers $(t...)")

Base.getindex(into::Tuple{}, index::Tuple{}) = ()
Base.getindex(into::Tuple, index::Tuple{}) = tuple_mismatch_error(into)
Base.getindex(into::Tuple{}, index::Tuple) = tuple_mismatch_error(index)
Base.getindex(into::Tuple, index::Tuple) = begin
    next = getindex(tail(into), tail(index))
    ifelse(first(index), (first(into), next...), next)
end

@code_warntype getindex((1, "a", 3), (Val{true}(), Val{false}(), Val{true}()))

@bramtayl
Copy link
Copy Markdown
Contributor Author

My follow-up pull request was going to be a type stable getindex and setindex for tuples.

@bramtayl
Copy link
Copy Markdown
Contributor Author

Both of which will be needed for a type-stable mapslices.

@Cody-G
Copy link
Copy Markdown
Contributor

Cody-G commented Sep 11, 2017

I've also worked on this before, and I had trouble finding a way to get constant folding to work in the way that's necessary for a package like @bramtayl's JuliennedArrays (which already works well). I've also read the manual and any relevant issues/discourse discussions that I can find, and I'm very interested to learn what is the proper way to do this with Julia v0.7 master.

@bramtayl
Copy link
Copy Markdown
Contributor Author

bramtayl commented Sep 13, 2017

Constant folding seems very incomplete. Here's another example where constant folding is not working as expected so value wrapping is necessary:

devalue(::Val{T}) where T = T
value_wrap(f) = (x, args...) -> Val(f(devalue(x), args...))

fieldtypes(any) = fieldtypes(Val(any))
function fieldtypes(val_type::Val)
    Base.@pure inner_function(i) = value_wrap(fieldtype)(val_type, i)
    ntuple(inner_function, value_wrap(nfields)(val_type))
end

# doesn't constant fold
fieldtypes(atype) =
    ntuple(nfields(atype)) do
        fieldtype(atype, i)
    end

@bramtayl
Copy link
Copy Markdown
Contributor Author

Am I correct in my understanding that constant propagation cannot survive passing through functions not known to the compiler as pure? If so, it severely limits the usefulness of constant propagation.

@bramtayl
Copy link
Copy Markdown
Contributor Author

My understanding is that this is a non-issue with the now aggressive constant propogation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs tests Unit tests are required for this change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants