allow passing a custom pmap
#63
Conversation
| @@ -157,7 +157,7 @@ function optimize(ho::Hyperoptimizer) | |||
| if e isa InterruptException | |||
| @info "Aborting hyperoptimization" | |||
| else | |||
| rethrow(e) | |||
There was a problem hiding this comment.
this will mess with the stacktrace and point to this line instead of the original one, so I fixed it to just rethrow() which does the right thing in a catch block
There was a problem hiding this comment.
Aha, I learned something new 😃
src/Hyperopt.jl
Outdated
| res = $(esc(ex.args[2])) # ex.args[2] = Body of the For loop | ||
|
|
||
| res, $(Expr(:tuple, esc.(params[2:end])...)) | ||
| put!(new_history, (i, res, $(Expr(:tuple, esc.(params[2:end])...)))) |
There was a problem hiding this comment.
here, we update new_history (which lives on the manager node) from the worker nodes, so that we don't need to care about the result of pmap. Why? Because if we pass a custom on_error, it would need to return both a loss and the parameters in order to be iterated later, and the user probably doesn't know they can't just pass a loss.
There was a problem hiding this comment.
Does this comment perhaps deserve to be included in the source as well? I can imagine future me scratching my head over this (Hyperopt.jl has a tendency to require a long warm up before I can do any maintainance on it 🙃)
| @@ -157,7 +157,7 @@ function optimize(ho::Hyperoptimizer) | |||
| if e isa InterruptException | |||
| @info "Aborting hyperoptimization" | |||
| else | |||
| rethrow(e) | |||
There was a problem hiding this comment.
Aha, I learned something new 😃
src/Hyperopt.jl
Outdated
| res = $(esc(ex.args[2])) # ex.args[2] = Body of the For loop | ||
|
|
||
| res, $(Expr(:tuple, esc.(params[2:end])...)) | ||
| put!(new_history, (i, res, $(Expr(:tuple, esc.(params[2:end])...)))) |
There was a problem hiding this comment.
Does this comment perhaps deserve to be included in the source as well? I can imagine future me scratching my head over this (Hyperopt.jl has a tendency to require a long warm up before I can do any maintainance on it 🙃)
|
one sec, I had a better idea for how to do this 😄 |
|
I pushed a new commit that takes a better approach I think— instead of syncing the history and results with a channel, it does the whole hyperoptimizer instead. I think that will allow workers to observe the history to choose iterates with that knowledge for any samplers that use that. I think an even better approach might Two tests are failing locally so I’ll have to look more at why tomorrow. Edit: they aren’t failing here so maybe I fixed it? Could’ve been a revise issue, since the workers don’t always like revise edit edit: they didn’t run at all, I’m just confused lol |
|
@omus I'd be interested in your thoughts on this new approach if you get a chance to have a look (using a RemoteChannel to access the single hyperoptimizer object, instead of using a RemoteChannel to store the history and results, but having one hyperoptimizer per worker). |
|
@baggepinnen I'll ask for a re-review if you get a chance, since I ended up changing this a fair bit since you reviewed. The new version seems to fix #62 though, and I was able to do a big 5-6 day long training job with this (only 12 iterations but each one is slow...). |
|
Cool approach! Thanks for fixing Eric 😃 New release will be out soon |
|
awesome, thanks! |
closes #59
with thanks to @omus who helped me figure out a path here.
I think
@phyperoptneeds more correctness work to go beyond simple samplers like the RandomSampler, but this at least lets us use the RandomSampler robustly by passing e.g. aParallelism.robust_pmapor such.