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

Corrections for Julia 0.6.0 #13

Merged
merged 7 commits into from
Jul 20, 2017
Merged

Conversation

Goysa2
Copy link
Contributor

@Goysa2 Goysa2 commented Jul 19, 2017

I've made some changes so that the performance profiles appear correctly.
Basically, NaN have been changed to Inf and then to NaN again. Now it outputs the same things that it did in Julia 0.5.2.

@@ -13,21 +13,22 @@ function performance_ratios(T :: Array{Float64,2}; logscale :: Bool=true)

(np, ns) = size(T); # Number of problems and number of solvers.

T[isinf(T)] = NaN;
T[T .< 0] = NaN;
T[isinf.(T)] = Inf;
Copy link
Member

Choose a reason for hiding this comment

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

Is this one really necessary then? Does it do anything?!

Copy link
Member

Choose a reason for hiding this comment

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

I think is should be T[isnan.(T)] = Inf? This way, maximum is also kept.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right it should be isnan instead of is inf. But I don't see why maximum should be kept. At least on my tests with isnan the maximum function still produce bad performance profiles

logscale && (r = log2(r));
max_ratio = maximum(r);
logscale && (r = log2.(r));
max_ratio = findmax(r)[1]
Copy link
Member

Choose a reason for hiding this comment

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

Was maximum not working?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Julia 0.5.2 maximum([2.0, NaN]) returned 2.0 and findmax([2.0, NaN])[1] returned 2.0 as well. But in Julia 0.6.0 maximum([2.0, NaN]) returns NaN and findmax([2.0, NaN])[1] returns 2.0. That's why I changed it.

Copy link
Member

Choose a reason for hiding this comment

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

@dpo dpo mentioned this pull request Jul 19, 2017
@dpo
Copy link
Member

dpo commented Jul 19, 2017

Thanks. We also need to add NaNMath to REQUIRE.

@Goysa2
Copy link
Contributor Author

Goysa2 commented Jul 19, 2017

I've added NaNMath in REQUIRE!

REQUIRE Outdated
@@ -1,2 +1,5 @@
julia 0.5
Plots

julia 0.6
Plots, NaNMath
Copy link
Member

Choose a reason for hiding this comment

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

Just add NaNMath to the list on line 3.

@dpo dpo closed this Jul 20, 2017
@dpo dpo reopened this Jul 20, 2017
@dpo
Copy link
Member

dpo commented Jul 20, 2017

ps: the failure on Windows 32 bit is known and fixed in 0.7-dev : JuliaPlots/Plots.jl#968. It may be backported to Julia 0.6?!

@dpo dpo merged commit 630816c into JuliaSmoothOptimizers:master Jul 20, 2017
@dpo
Copy link
Member

dpo commented Jul 20, 2017

Thank you!

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.

3 participants