-
Notifications
You must be signed in to change notification settings - Fork 5
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
257 Adding averaging time using timeit #285
Changes from 13 commits
cd9ad1b
d86fab1
51728ef
babfa2b
afa69b8
986ec5d
433321a
f0c20c2
2c68507
c9c808f
1c31a9d
cc88b5a
30dc3f7
84e0eee
3831ce5
057e44e
56d194f
c750fb2
ac4745c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
|
||
from __future__ import absolute_import, division, print_function | ||
|
||
import time | ||
import timeit | ||
|
||
import numpy as np | ||
|
||
|
@@ -32,17 +32,23 @@ | |
ScipyController = None | ||
|
||
|
||
def fitbm_one_prob(user_input, problem): | ||
def fitbm_one_prob(user_input, problem, num_runs): | ||
""" | ||
Sets up the controller for a particular problem and fits the models | ||
provided in the problem object. The best fit, along with the data and a | ||
starting guess is then plotted on a visual display page. | ||
|
||
@param user_input :: all the information specified by the user | ||
@param problem :: a problem object containing information used in fitting | ||
|
||
@returns :: nested array of result objects, per function definition | ||
containing the fit information | ||
:param user_input :: all the information specified by the user | ||
:type user_input :: UserInput | ||
:param problem :: a problem object containing information used in fitting | ||
:type problem :: FittingProblem | ||
:param num_runs :: number of times controller.fit() is run to | ||
generate an average runtime | ||
:type num_runs :: int | ||
|
||
:return :: nested array of result objects, per function definition | ||
containing the fit information | ||
:rtype :: list | ||
""" | ||
|
||
results_fit_problem = [] | ||
|
@@ -58,7 +64,8 @@ def fitbm_one_prob(user_input, problem): | |
if software in controllers: | ||
controller = controllers[software](problem, user_input.use_errors) | ||
else: | ||
raise NotImplementedError('The chosen software is not implemented yet: {}'.format(user_input.software)) | ||
raise NotImplementedError('The chosen software is not implemented yet:' | ||
'{}'.format(user_input.software)) | ||
|
||
# The controller reformats the data to fit within a start- and end-x bound | ||
# It also estimates errors if not provided. | ||
|
@@ -71,7 +78,8 @@ def fitbm_one_prob(user_input, problem): | |
controller.function_id = i | ||
|
||
results_problem, best_fit = benchmark(controller=controller, | ||
minimizers=user_input.minimizers) | ||
minimizers=user_input.minimizers, | ||
num_runs=num_runs) | ||
|
||
if best_fit is not None: | ||
# Make the plot of the best fit | ||
|
@@ -85,47 +93,64 @@ def fitbm_one_prob(user_input, problem): | |
return results_fit_problem | ||
|
||
|
||
def benchmark(controller, minimizers): | ||
def benchmark(controller, minimizers, num_runs): | ||
""" | ||
Fit benchmark one problem, with one function definition and all | ||
the selected minimizers, using the chosen fitting software. | ||
|
||
@param controller :: The software controller for the fitting | ||
:param controller :: The software controller for the fitting | ||
:type controller :: Object derived from BaseSoftwareController | ||
@param minimizers :: array of minimizers used in fitting | ||
:type minimizers :: list | ||
@param num_runs :: number of times controller.fit() is run to | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the docstring standardisation should include types for parameters (see #236), particularly if functions are part of the API (which this might be). Probably a good opportunity to add types for existing parameters as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have gone through There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a bit of a tricky one using the sphinx docstring style, as it doesn't deal with multiple return types as well as the numpy style. However I would be inclined to either put There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good to me. |
||
generate an average runtime | ||
:type num_runs :: int | ||
|
||
|
||
@returns :: nested array of result objects, per minimizer | ||
and data object for the best fit data | ||
:return :: tuple(results_problem, best_fit) nested array of | ||
result objects, per minimizer and data object for | ||
the best fit data | ||
:rtype :: (list of FittingResult, plot_helper.data instance) | ||
""" | ||
min_chi_sq, best_fit = None, None | ||
results_problem = [] | ||
|
||
for minimizer in minimizers: | ||
controller.minimizer = minimizer | ||
|
||
controller.prepare() | ||
|
||
init_function_def = controller.problem.get_function_def(params=controller.initial_params, | ||
function_id=controller.function_id) | ||
init_function_def = controller.problem.get_function_def( | ||
params=controller.initial_params, | ||
function_id=controller.function_id) | ||
try: | ||
start_time = time.time() | ||
controller.fit() | ||
end_time = time.time() | ||
except Exception as e: | ||
print(e.message) | ||
# Calls timeit repeat with repeat = num_runs and number = 1 | ||
runtime_list = \ | ||
timeit.Timer(setup=controller.prepare, | ||
stmt=controller.fit).repeat(num_runs, 1) | ||
runtime = sum(runtime_list) / num_runs | ||
|
||
# Catching all exceptions as this means runtime cannot be calculated | ||
# pylint: disable=broad-except | ||
except Exception as excp: | ||
print(str(excp)) | ||
controller.success = False | ||
end_time = np.inf | ||
|
||
runtime = end_time - start_time | ||
runtime = np.inf | ||
|
||
controller.cleanup() | ||
|
||
fin_function_def = controller.problem.get_function_def(params=controller.final_params, | ||
function_id=controller.function_id) | ||
fin_function_def = controller.problem.get_function_def( | ||
params=controller.final_params, | ||
function_id=controller.function_id) | ||
|
||
if not controller.success: | ||
chi_sq = np.nan | ||
status = 'failed' | ||
else: | ||
cv = np.std(runtime_list) / np.mean(runtime_list) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apologies, as my original suggestion was a bad metric for determining whether caching has occurred. It would be better to test the ratio of the max to the min (as if a large number of runs were cached then the stdefv could be low despite a single outlier). I've had a look at the timeit source code for Cython and the ratio they use is 4, so we could pick this to be consistent. Also I think rather than raising the Warning maybe we should use
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
if cv > 0.2: | ||
raise Warning('The ratio of the standard deviation and mean' | ||
' is larger than {}, this may indicate caching' | ||
' is used in the timing results.'.format()) | ||
|
||
chi_sq = misc.compute_chisq(fitted=controller.results, | ||
actual=controller.data_y, | ||
errors=controller.data_e) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,5 +12,7 @@ | |
"ralfit" : ["ralfit"] | ||
}, | ||
|
||
"comparison_mode" : "both" | ||
"comparison_mode" : "both", | ||
|
||
"num_runs" : 5 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this pattern need to be included here as well as in
fitbenchmark_group
? Is it not dealt with there if the user doesn't define it correctly here? If it is required then maybe it would be worth adding a comment or two explaining what it does?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From our meeting on Friday I thought the idea was to remove the expert scirpt so this would not be a problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's fine.