ML-Plan: Added prediction runtime for test predictions. #238
Conversation
mwever
changed the title
frameworks/MLPlan/exec.py
Outdated
| @@ -85,7 +85,8 @@ def run(dataset, config): | |||
|
|
|||
| predictions = stats["predictions"] | |||
| truth = stats["truth"] | |||
| numEvals = stats["num_evaluations"] | |||
| num_evals = stats["num_evaluations"] | |||
| predict_time = stats["final_candidate_predict_time_ms"] | |||
There was a problem hiding this comment.
Could you please use a safe way to add this information so it does not break older versions of ML-Plan?
frameworks/MLPlan/exec.py
Outdated
| @@ -102,8 +103,9 @@ def run(dataset, config): | |||
| probabilities=probabilities, | |||
| probabilities_labels=probabilities_labels, | |||
| target_is_encoded=is_classification, | |||
There was a problem hiding this comment.
It looks like the target is not (no longer?) encoded. Running python runbenchmark.py MLPlan openml/t/59 -f 0 fails due to predictions being a list with string labels (e.g. Iris-Setosa) while target_is_encoded is set to True.
|
@mwever do you have time to make the suggested changes? It shouldn't take long. If not then I can probably do it, but it's a bit messy since I can't push to your fork. |
PGijsbers
added
the
framework
|
Hey @PGijsbers! |
|
Thanks a lot! From a glance it looks good 👍 I'll verify (and hopefully merge) on Monday |
|
On my machine ML-Plan fails on the APSFailure dataset, tested with Is this reproducible for you? Could you see if you can fix this? Full trace: |
|
I just noticed I had modified my |
|
Hey, |
|
This is how that is solved with Auto-WEKA. Adding it to ML-Plan does seem to give some issues (because it looks like benchmark dependencies are not installed). I could look into it later. |
|
Thank you a lot, @PGijsbers |
|
My bad, still learning 😓 the reason it works for Auto-WEKA is that it doesn't run in its own virtual environment. |
|
Got it. So, the problem is that there is no actual |
From the docs, but obviously I am not aware of a possibility to set a definition as abstract. To prevent the configuration files (and their parsing) from becoming too complicated, I think I would favor the @sebhrusen your thoughts? |
|
Oh, I thought that the
@PGijsbers yeah, we probably need to be more explicit now about what is allowed and what is not by default in the
what is not allowed?
For the high-level parameter, did we agree that we allow only one additional definition with a non-default mode/preset? |
|
Okay, sorry for the confusion! Writing an answer to @sebhrusen 's comment, I just noticed that the ckeck in To conclude, ML-Plan does not need any default parameter anymore in the resources file. |
|
Verified ML-Plan runs successfully on all validation tasks now. @sebhrusen What's the policy on saving artifacts now? H2O produces a |
There's no strict policy, mainly a concern regarding what is uploaded to s3 / downloaded to benchmark runner by default.
To zip a "filtered" directory stucture or apply a function to it, from frameworks.shared.callee import utils
if 'models' in artifacts:
shutil.rmtree(os.path.join(predictor.path, "utils"), ignore_errors=True)
models_dir = output_subdir("models", config)
utils.zip_path(predictor.path, os.path.join(models_dir, "models.zip"))
… |
@mwever the definition files with a versioned name/time should not be modified, so you did right.
Is it only since v0.2.4? By default parameter, you mean default/abstract definition? |
|
about the |
|
@mwever I'm willing to merge this PR, so to try it locally, I rebased it on top of However if the benchmark was working fine with your version I can still merge this PR and let you fix this on a separate PR if you want. |
|
I am a bit confused, |
|
Looks to me as if there is an issue with the scikit-learn backend wrapper. I will fix that issue asap. |
|
@mwever do you have an estimate of when you'll be able to add the fix? |
|
@PGijsbers, @mwever I already agreed to the merge, I'll let the decision to the PR author. |
|
Meanwhile, I agree that it might be better to merge the PR already. It could be that I can fix the issue this or next week but I currently cannot give any guarantees on the time. However, since the WEKA backend is working right now and we previously agreed on only benchmarking the WEKA backend for now, I hope it is fine if we do the sklearn backend in another PR. So, please go ahead and merge this one :-) |
Note: ML-Plan takes the time for fitting the finally chosen solution candidate into account and tries to meet the timeout including fitting the solution. The prediction time for the test data, however, is not anticipated.
Furthermore, we just released a new version (0.2.4) with several bug fixes and a few enhancements. I did not want to mess up the frameworks.yaml files so I did not touch them. As far as I can see, except for the Q2.2020 file, the version of ML-Plan always refers to the "latest" version. If this is also the version which is benchmarked everything should be fine.