Implementing channel control for experiment creation #41
Conversation
…rect concurrency workflow
| @@ -19,6 +19,15 @@ function log_evaluation(logger::Logger, performance_evaluation) | |||
| updaterun(logger.service, run, "FINISHED") | |||
| end | |||
|
|
|||
| function log_evaluation(logger::Logger, performance_evaluation) | |||
There was a problem hiding this comment.
Perhaps this function should close result_channel before returning?
There was a problem hiding this comment.
The result_channel is just temporal. When we perform take!(result_channel), the value is retrieved and returned. The garbage collector will take care off the channel when function ends, so it's safe to maintain it open.
Also, we are not looping the result_channel in any way, so we don't have to worry about that.
(I can remove the wait(result_channel) at line 26 because take!(result_channel) will actually wait until the Channel contains a value)
| @@ -28,13 +28,17 @@ struct Logger | |||
| verbosity::Int | |||
| experiment_name::String | |||
| artifact_location::Union{String,Nothing} | |||
| _logging_channel::Channel{Tuple} | |||
There was a problem hiding this comment.
Nit picking point: Can't _logging_channel be logging_channel ?
There was a problem hiding this comment.
I considered _logging_channel instead of logging_channel because I want it to be like a "private" field. The user is not intended to touch that field. However, I discovered that I made a mistake on open_logging_channel() because I'm returning a value instead of setting it up in the logger.
There was a problem hiding this comment.
I think in Julia fields are generally considered private, especially if they are not referenced in any doc-string. To make them public you would ordinarily provide an accessor function. But there's no harm in this - I just found it a bit strange.
Opening this PR in favor of
Channelusage. It requires this MLJTuning.jl branch to be merged and released in a new patch.The complete discussion is here #36