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

Not working with simple example #16

Closed
juliohm opened this issue Jun 4, 2017 · 6 comments
Closed

Not working with simple example #16

juliohm opened this issue Jun 4, 2017 · 6 comments

Comments

@juliohm
Copy link

juliohm commented Jun 4, 2017

Could you please clarify what is accepted as argument to @bench, this is another package for which the functionality is not working:

using GeoStats
using PkgBenchmark

srand(2017)

dim = 3; nobs = 100
X = rand(dim, nobs); z = rand(nobs)
xₒ = rand(dim)
γ = GaussianVariogram(1., 1., 0.)

@benchgroup "Kriging" ["kriging"] begin
  @bench "SimpleKriging" simkrig = SimpleKriging(X, z, γ, mean(z))
  @bench "OrdinaryKriging" ordkrig = OrdinaryKriging(X, z, γ)
  @bench "UniversalKriging" unikrig = UniversalKriging(X, z, γ, 1)
  for j=1:nobs
    @bench "SimpleKrigingEstimate" estimate(simkrig, X[:,j])
    @bench "OrdinaryKrigingEstimate" estimate(ordkrig, X[:,j])
    @bench "UniversalKrigingEstimate" estimate(unikrig, X[:,j])
  end
end
@shashi
Copy link
Collaborator

shashi commented Jun 5, 2017

I think for now, you have to use all arguments with a $ prefix. Try

using GeoStats
using PkgBenchmark

srand(2017)

dim = 3; nobs = 100
X = rand(dim, nobs); z = rand(nobs)
xₒ = rand(dim)
γ = GaussianVariogram(1., 1., 0.)

@benchgroup "Kriging" ["kriging"] begin
  @bench "SimpleKriging" simkrig = SimpleKriging($X, $z, $γ, mean($z))
  @bench "OrdinaryKriging" ordkrig = OrdinaryKriging($X, $z, $γ)
  @bench "UniversalKriging" unikrig = UniversalKriging($X, $z, $γ, 1)
  for j=1:nobs
    @bench "SimpleKrigingEstimate" estimate(simkrig, $X[:,$j])
    @bench "OrdinaryKrigingEstimate" estimate(ordkrig, $X[:,$j])
    @bench "UniversalKrigingEstimate" estimate(unikrig, $X[:,$j])
  end
end

This will create closures with the value of the variables from the defining scope. If you don't do this, it will just look for the latest values after the benchmarks file is completely loaded.

Note that @bench doesn't run the benchmark at the point of definition, it only adds a definition to a global state object. PkgBenchmark then runs the benchmark from this state.

@jrevels
Copy link
Member

jrevels commented Jun 5, 2017

This is a symptom of the same transparency problem I discovered with the old BenchmarkTrackers macros (BenchmarkTrackers was the prototype for BenchmarkTools). See my comment here.

The solution was to get rid of the macros in favor of a Dict-based redesign that avoids special syntax or magic. Painting over that solution with more macros feels like a step backwards to me...

@juliohm
Copy link
Author

juliohm commented Jun 6, 2017

@shashi unfortunately it doesn't work. I tried copying/pasting it and I got new errors saying that simkrig is not defined.

@Evizero
Copy link

Evizero commented Jul 14, 2017

I think the problem here is that you try feeding values produced in one benchmark as input to another. You are probably better off with something like

@bench "SimpleKrigingEstimate" estimate($(SimpleKriging(X, z, γ, mean(z))), $(X[:,j]))

@juliohm
Copy link
Author

juliohm commented Jul 14, 2017

Thank you @Evizero, I appreciate your help. I am not using PkgBenchmark.jl in my projects anymore, but it is good to have something that works for future readers.

@juliohm
Copy link
Author

juliohm commented Dec 1, 2017

This issue is not valid anymore given the recent fixes by @KristofferC in the master branch. Closing it.

@juliohm juliohm closed this as completed Dec 1, 2017
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

4 participants