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

Variable initialisation #170

Closed
dmey opened this issue Oct 28, 2022 · 5 comments
Closed

Variable initialisation #170

dmey opened this issue Oct 28, 2022 · 5 comments
Labels
structure 🏠 Internal structure of the code style 💄 Coding but in style

Comments

@dmey
Copy link
Contributor

dmey commented Oct 28, 2022

Variables (e.g. column_variables.jl) are initialised with zero. Is there any reason for doing so? If not why not using NaN for floats and typemax or typemin for ints?

@milankl
Copy link
Member

milankl commented Nov 1, 2022

Does it matter, it will be overwritten anyway? To initialise arrays as with NaNs we would need to do

julia> fill(convert(NF,NaN),2,3)
2×3 Matrix{Float16}:
 NaN  NaN  NaN
 NaN  NaN  NaN

for example, such that for ever NF<:AbstractFloat there needs to a conversion from NaN to whatever NaN corresponds to in NF.

@milankl milankl added style 💄 Coding but in style structure 🏠 Internal structure of the code labels Nov 1, 2022
@milankl milankl added this to the v0.5 milestone Nov 1, 2022
@dmey
Copy link
Contributor Author

dmey commented Nov 7, 2022

Sometime is hard to distinguish unset or improperly used variables down at the bottom of the stack and for debugging and assertions having NaNs helps... If there is no difference in terms of performance etc, I think it would be safer to use NaNs where possible.

@milankl
Copy link
Member

milankl commented Nov 8, 2022

No it's not a performance issue. In both cases memory is allocated and filled with bits. Doesn't matter what those are.

julia> using BenchmarkTools
julia> c = NaN
julia> @btime fill($c,1000,1000);
  1.160 ms (2 allocations: 7.63 MiB)

julia> c = 0.0
julia> @btime fill($c,1000,1000);
  1.163 ms (2 allocations: 7.63 MiB)

Note that in the dynamics we allocate with zeros, and I never had a debugging-related issue with this. Happy for you to convert the initialisation for ColumnVariables to init with NaN, if people prefer that. I would suggest to define

function nans(::Type{T},dims...) where T
     return fill(convert(T,NaN),dims...)
end

in utility_functions.jl such that zeros(NF,...) works as nans(NF,...)

julia> nans(Float16,2)
2-element Vector{Float16}:
 NaN
 NaN

julia> nans(Float32,2)
2-element Vector{Float32}:
 NaN
 NaN

@milankl
Copy link
Member

milankl commented Nov 8, 2022

nans has been added now ☝🏼

@dmey
Copy link
Contributor Author

dmey commented Nov 8, 2022

@milankl great, thanks. Will add this to my current PRs. Feel free to close.

@milankl milankl closed this as completed Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
structure 🏠 Internal structure of the code style 💄 Coding but in style
Projects
None yet
Development

No branches or pull requests

2 participants