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

Remove Int64 type definition from Problem #115

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ovainola
Copy link
Contributor

Problem function defined dimension as an Int64 type, and this
restricts using JuliaFEM on 32bit architecture. Changed type to
a more generic: Integer.

fixes issue #113

Problem function defined dimension as an Int64 type, and this
restricts using JuliaFEM on 32bit architecture. Changed type to
a more generic: Integer.

fixes issue JuliaFEM#113
@coveralls
Copy link

Coverage Status

Coverage decreased (-3.03%) to 87.172% when pulling b520af3 on ovainola:master into aff194b on JuliaFEM:master.

@ahojukka5
Copy link
Member

We are also having problems with Float64 if we want to have JuliaFEM to work on 32 bit machines.

julia> f(x::Float32) = 2x
f (generic function with 2 methods)

julia> f(2.0)
ERROR: MethodError: no method matching f(::Float64)
Closest candidates are:
  f(::Float32) at REPL[3]:1
  f(::Int32) at REPL[1]:1

In general, should we use AbstractFloat instead of Float64 and Integer instead of Int64 to have also 32-bit compability?

@ovainola
Copy link
Contributor Author

it'd be more general if we would change.

@ChrisRackauckas
Copy link

No. Float64 is the default float on 32-bit machines and 64-bit machines. The difference is only in integer sizes. However, instead of using the abstract Integer, you should probably be using Int. This is an alias, and on 32-bit Int == Int32 and on 64-bit Int == Int64. So then typeof(1) == Int is true on either machine. You should then, in the few cases where you expect overflow (the integer is greater than 2147483647) upconvert inside of your functions to Int64 manually via Int64(a).

@ChrisRackauckas
Copy link

Oh, I misread this at first. Yes, you won't have performance issues if you loosen function definitions, so it's usually recommended you go to Integer and AbstractFloat, or whatever is the highest abstract type which makes sense. For storing the values in types, you should make them concrete, so either parameterize or use the concrete types Float64 and Int as explained above.

return Problem{P}(name, dimension, "none", [], Dict(), Assembly(), Dict(), Vector(), P())
end
function Problem{P<:FieldProblem}(::Type{P}, dimension::Int64)
function Problem{P<:FieldProblem, I<:Integer}(::Type{P}, dimension::I)

Choose a reason for hiding this comment

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

you can make this dimension::Integer to make it more succinct.

@ahojukka5
Copy link
Member

This particular file has been fixed in FEMBase, however, there's still use of Int64 in code, which should be replaced with Int. In order to make progress, we should activate Appvoyer to test in 32-bit environment to catch all problems with type definitions. See this PR (JuliaFEM/FEMBase.jl#46).

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

Successfully merging this pull request may close these issues.

4 participants