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

Data interoperability between 64-bit systems and 32-bit systems #162

Closed
kimikage opened this issue Jan 11, 2020 · 7 comments
Closed

Data interoperability between 64-bit systems and 32-bit systems #162

kimikage opened this issue Jan 11, 2020 · 7 comments

Comments

@kimikage
Copy link
Collaborator

BTW, I think there is a problem in interoperability with specialization using integer literal parameters.

julia> Normed{UInt32,Int64(1)} === Normed{UInt32,Int32(1)}
false

julia> Normed{UInt32,Int64(1)} == Normed{UInt32,Int32(1)}
false

I don't know how serializers (e.g. JLD2) do (de-)serialization.

Originally posted by @kimikage in #129 (comment)

I think f <: Integer is fine except for the ambiguity associated with omitting constraints, but I think isa(f, Int) is problematic as a constraint.

isa(f, Int) || error("f must be an Int")

Originally posted by @kimikage in #155 (comment)

For example:

julia> versioninfo()
Julia Version 1.0.5
Commit 3af96bcefc (2019-09-09 19:06 UTC)
Platform Info:
  OS: Windows (i686-w64-mingw32)
  CPU: Intel(R) Core(TM) i7-8565U CPU @ 1.80GHz
  WORD_SIZE: 32
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.0 (ORCJIT, skylake)

julia> using JLD2, FileIO, TestImages

julia> img = testimage("cameraman");

julia> @save "cameraman.jld2" img
julia> versioninfo()
Julia Version 1.3.1
Commit 2d5741174c (2019-12-30 21:36 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i7-8565U CPU @ 1.80GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.1 (ORCJIT, skylake)

julia> using JLD2, FileIO, ColorTypes

julia> @load "cameraman.jld2" img;

julia> typeof(img)
Array{Gray{Normed{UInt8,8}},2}

julia> img[1,1]
Gray{N0f8}(Error showing value of type Gray{Normed{UInt8,8}}:
ERROR: f must be an Int

Of course, there is no need to save 2D images in JLD2. This is just an example.

Most functions should work with f in both Int32 and Int64 for now, since I avoided specialization with integer literals. The only specialization with integer literals is:

N0f16(x::N0f8) = reinterpret(N0f16, convert(UInt16, 0x0101*reinterpret(x)))

Perhaps adding promotion rules (Int32 or Int64 -> Int) will complete the interoperability. Well, we don't aim to achieve the interoperability, though.

Originally posted by @kimikage in #155 (comment)

On the other hand:

I do think forcing Int is a useful normalization:

julia> using FixedPointNumbers

julia> T1 = Normed{UInt8,8}
Normed{UInt8,8}

julia> T2 = Normed{UInt8,Int32(8)}
Normed{UInt8,8}

julia> T1 == T2
false

julia> x = T2(0.4)
0.4N0f8

julia> isa(x, N0f8)
false

That's really confusing.

Originally posted by @timholy in #155 (comment)

@kimikage
Copy link
Collaborator Author

I'm embarrassed to say, but I learned that the constructors can return instances of types other than their own type, after reporting this issue,
https://docs.julialang.org/en/v1/manual/conversion-and-promotion/#Conversion-vs.-Construction-1

Not returning instances of their own types might cause a bug. Also, the users and other packages may use convert instead of the constructor for the instance generation.

@timholy
Copy link
Member

timholy commented Mar 14, 2020

Not returning instances of their own types might cause a bug.

For my own coding I've adopted the viewpoint that when you might want something like the Flip example on that page, write it as flip (a function call rather than constructor) and put the dispatch logic into flip rather than Flip. The Some example, however, is fine.

ImageCore used to have a type called ColorView (before we switched to using ReinterpretArray, which has turned out to be less than ideal), but there was also a colorview function which might or might not create a ColorView depending on the input types.

@kimikage
Copy link
Collaborator Author

put the dispatch logic into flip rather than Flip

In general, I think it is a good design. However, that is not a solution for this problem.
My (original) plan allowed the ::Int32 versions and the ::Int64 versions to "coexist". However, you said:

IMO any time there's an integer type parameter it should just be ::Int.

Originally posted by @timholy in #155 (comment)

@timholy
Copy link
Member

timholy commented Mar 14, 2020

I'm a big fan of the constructor throwing an error if it's not Int.

@kimikage
Copy link
Collaborator Author

That is one of the reasonable plans. However, if you introduce the change, the special handling of FixedPoint may "diffuse" to other packages (e.g. JLD2).

@timholy
Copy link
Member

timholy commented Mar 14, 2020

the special handling of FixedPoint may "diffuse" to other packages (e.g. JLD2).

Yep, I'm aware and glad you noticed this. JuliaLang/julia#34977. I'm not quite sure what the best policy would be, but perhaps a normalize_parameters function could be defined in an IOCore package that then gets loaded by HDF5, JLD2, etc. FPN could depend on IOCore and specialize it for FixedPoint.

@kimikage
Copy link
Collaborator Author

kimikage commented Apr 6, 2024

Perhaps this no longer seems to be a practical issue.

@kimikage kimikage closed this as completed Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants