-
Notifications
You must be signed in to change notification settings - Fork 237
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
[Exhaustive Tuning] Search failed Error message when no solution is available #2253
Comments
Hi @junliume could you please give a command line to reproduce this? |
@dmikushin it might be too heavy to reproduce with migraphx online tuning. I suggest doing static code check on the file: |
@dmikushin One can make a fake tunable solver that is applicable in every net config and has 0 perf configs. That would be a good start for a unit test. |
@dmikushin this issue has raised urgency level due to request to cherry pick into existing release. Please let me know if you have a plan to fix it. Thanks! CC: @JehandadKhan |
Hi @junliume yes, I'm working on it. |
@junliume , something like this? As I understand the logic, we could still allow the function to execute till the end for simplicity, but do not fail it if there were no runs. diff --git a/src/include/miopen/generic_search.hpp b/src/include/miopen/generic_search.hpp
index 5fb60235d..be6d89f58 100644
--- a/src/include/miopen/generic_search.hpp
+++ b/src/include/miopen/generic_search.hpp
@@ -212,7 +212,7 @@ public:
n_recent != 0u ? (static_cast<float>(n_total - n_recent) *
(elapsed_cumulative / static_cast<float>(n_recent)) / 1000.0f)
: 0.0f; // paraniod
- MIOPEN_LOG_W(n_recent << '/' << n_failed << '/' << n_total << ' ' << total_best
+ MIOPEN_LOG_I(n_recent << '/' << n_failed << '/' << n_total << ' ' << total_best
<< ", best within recent " << n_within_beat << ": " << best_time
<< " #" << n_best << ' ' << best_config << ", ETA:" << eta_sec
<< " sec.");
@@ -275,7 +275,7 @@ auto GetAllConfigs(const Solver s, const Context& context, const Problem& proble
ComputedContainer<PerformanceConfig, Context, Problem> all_configs = useSpare ? spare : primary;
const int n_runs_total = useSpare ? spare_size : primary_size;
- MIOPEN_LOG_W(s.SolverDbId() << ": Searching the best solution among " << n_runs_total
+ MIOPEN_LOG_I(s.SolverDbId() << ": Searching the best solution among " << n_runs_total
<< (useSpare ? " (spare)" : "") << "...");
return all_configs;
@@ -517,7 +517,7 @@ auto GenericSearch(const Solver s,
}
}
- // Banchmarked kernels will not be used anymore.
+ // Benchmarked kernels will not be used anymore.
// Now we can delete Program objects that belong to OCL/HIP
// runtime and free the associated resources (memory, file handles...)
for(const auto& kernelInfo : current_solution.construction_params)
@@ -548,10 +548,10 @@ auto GenericSearch(const Solver s,
for(auto& agent : compile_agents)
agent.join();
- MIOPEN_LOG_W("Done: " << n_runs_total << '/' << n_failed << '/' << n_runs_total << ", best #"
+ MIOPEN_LOG_I("Done: " << n_runs_total << '/' << n_failed << '/' << n_runs_total << ", best #"
<< n_best << ' ' << best_time << ' ' << best_config);
- if(!is_passed)
+ if(!is_passed && n_runs_total)
MIOPEN_THROW("Search failed");
// Run once with the default config and show score.
@@ -560,7 +560,7 @@ auto GenericSearch(const Solver s,
invoker(profile_h, invoke_ctx);
const auto default_time = profile_h.GetKernelTime();
const auto score = (best_time > 0.0f) ? default_time / best_time : 0.0f;
- MIOPEN_LOG_W("...Score: " << score << " (default time " << default_time << ')');
+ MIOPEN_LOG_I("...Score: " << score << " (default time " << default_time << ')');
return best_config;
} |
@JehandadKhan could you review the above proposal? It looks good to me. IMHO it's better to break early from the function execution but it also does not harm to run through the function without failure since these conditions appear rarely. |
@dmikushin let's go with the proposal, please form a PR and I can help to push it through. |
This solver would be incorrect. |
Please look at #2266 (comment) |
Some info about restrictions related to tunable solvers: #866 (comment) |
Not actually fixed yet. Continued in #2270... |
This looks like a tiny issue which could be a good first one for new players of MIOpen:
normally we would expect something like this:
A few issues:
0
solution is applicable to this case, we should have an early break in the search instead of emitting an error.information
rather thanwarning
.The text was updated successfully, but these errors were encountered: