Skip to content
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

The integration of SasView #199

Merged
merged 50 commits into from Sep 1, 2019
Merged

The integration of SasView #199

merged 50 commits into from Sep 1, 2019

Conversation

AtomAnu
Copy link
Contributor

@AtomAnu AtomAnu commented Aug 21, 2019

Description of Work

This is the integration of SasView. There are five main additions for this integration.

  1. An additional example script, example_runScripts_SasView.py has been added to use SasView as the software to fit the user-selected problem(s). At the start of the example script, there are three condition statements with each statement verifying if one of the three required SasView packages is installed. Bumps, sasmodels and sascalc are the three required packages. As explained in the FitBenchmarking README.md file, Bumps and sasmodels can be installed using pip while sascalc is not an independent package.

    Additionally, the package sasmodels contains a script data.py which contains a module called load_data. This module requires sascalc in order to function. In this module's code, the line of code which imports sascalc is written below.

    from sas.sascalc.dataloader.loader import Loader

    Furthermore, sascalc now exists under the directory src/sas which resides in the SasView repository (https://github.com/SasView/sasview)

    As a consequence, it was neccessary to copy the folder sas from the SasView repository and place it inside FitBenchmarking (fitbenchmarking/sas) in order to run SasView headless inside FitBenchmarking.

  2. A parsing script, parse_sasview_data.py has been added to parse SasView problem definition files and their corresponding data files. This script can later be integrated with parse_fitbenchmark_data.py.

    in parse_sasview_data.py, the eval_f and get_function modules work in the same manner as that of parse_nist_data.py and parse_fitbenchmark_data.py. However, the module eval_f for parse_sasview_data.py accepts the parameters list in two formats. The first format is a string which contains the parameter names and values. The second format is to accept the parameter values as multiple parameters to the module.

    In parse_fitbenchmark_data.py, a new module, get_bumps_function, is created to format Mantid functions into the functions that are acceptable by Bumps. Functions/models that are acceptable by Bumps should not contain any arguments (*arg) or keyword arguments (**kwargs).

  3. A data preparation script, sasview/prepare_data.py, has been created to prepare the data in the correct format. There are three main tasks done in this script. The first task is to set the data errors (data_e) if they are not given in the data file. The second task is to set the X data range if specified. If the X data range is not specified, the range would be set from the minimum X value to the maximum X value. The final task is to create a data object of type sasmodels.data.Data1D so that it can be passed onto Bumps.

  4. A function definition preparation script, sasview/func_def.py, has been created to prepare the models/functions and their starting parameter values for Bumps fitting. The script also prepares the initial and the final function definition strings for each fit.

  5. Finally, a script, sasview/main.py, has been added to perform fitting using Bumps. All the fitting occur in the module fit. One can observe that this module is quite lengthy. This is because the method of configuring the function/model is different for SasView models and other functions from Scipy and Mantid. Nevertheless, this module could be shorten up in the future.

    This script also calculates the chi-squared value and the run time of each fit. Then, a result object is created for the graph and table creating modules. A single result object contains the fitted Y values, the fit status, the chi-squared value, the fit runtime, the minimiser name, the initial function definition string and the final function definition string.


Note that the unit tests for these scripts have not been completed.

Testing Instructions

  1. run example_runScripts.py with problems of type NIST, FitBenchmark and SasView
  2. run example_runScripts_mantid.py with problems of type NIST, FitBenchmark and SasView
  3. run example_runScripts_SasView.py with problems of type NIST, FitBenchmark and SasView

…nchmarking

The file `example_runScripts_SasView.py` has been updated so that it will prompt a message if Bumps, sasmodels or sascalc is/are not installed.

The folder `sas/sascalc` is added as sascalc is not an independent package to be installed via `pip` yet. This folder should be removed once there is an installation method for sascalc.
@AtomAnu AtomAnu added the Enhancement New feature or request label Aug 21, 2019
A script to test the modules in `parse_sasview_data.py` has been added. A set of mock problem files has also been added.
This is a unit test file for `sasview/prepare_data.py`
This will be remove once bumps and sasmodels can be installed by `python setup.py install`.
lxml is required by sasmodels
@Anders-Markvardsen
Copy link
Contributor

@AtomAnu thanks for this PR and it detailed description. You write "As a consequence, it was neccessary to copy the folder sas from the SasView repository and place it inside FitBenchmarking (fitbenchmarking/sas) in order to run SasView headless inside FitBenchmarking.".

A questions:

  • In the sasview meeting tuesday did you discuss with the sasview team this aspects? - i.e. to include a copy of parts of there code until the sasview team has separated sascal out as separate package.
    • assuming this is OK then as a minimum please add readme in that folder to specify when the copy was made and a copy of your above reasoning for adding it.

Obviously the above is not ideal, but with the sasview team actively working on separating out sascal it seems like the best workaround

@Anders-Markvardsen
Copy link
Contributor

Thinking about it again. As an alternative to copy and paste have you considered code which

  • clone the sasview repo in read only mode, either a particular version of it directly or where after the repo has been cloned it is reset to a particular version. The latter is to guard against the expected future changes of this part of the sasview codebase
  • copy the sascal part, and this might be a tricky part: to a location where it is visible from the python path as appropriate
  • delete the cloned sasview repo

See https://stackoverflow.com/questions/2472552/python-way-to-clone-a-git-repository for suggestions for how a github repo can be cloned

README.md Outdated Show resolved Hide resolved
@AtomAnu
Copy link
Contributor Author

AtomAnu commented Aug 23, 2019 via email

return results_problem, best_fit

def fit(problem, data, function, minimizer, init_func_def):
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code comment please

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant def comment please

t_start, t_end = None, None
model = function[0]

if hasattr(model, '__call__'):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few code comments here may help


final_param_values = result.x

fin_func_def = get_fin_function_def(final_param_values, problem, init_func_def)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does 'fin' in fin_func_def stand for

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The word fin stands for final in this case.


from utils.logging_setup import logger

def prepare_data(problem, use_errors):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prepare data for what?

popt = list(popt)
params = init_function_def.split("|")[1]
params = re.sub(r"[-+]?\d+\.\d+", lambda m, rep=iter(popt):
params = re.sub(r"[-+]?\d+[.]\d+", lambda m, rep=iter(popt):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks subtle - code comment please and e.g. line 61 also


problem_type = extract_problem_type(problem)

if not 'name=' in str(problem.equation):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add comment to specify what use case this is. And for else below also

function_defs = [[fit_function, params]]

return function_defs

def get_fit_function_without_kwargs(fit_function, functions_string):
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add short def description

def get_fit_function_without_kwargs(fit_function, functions_string):
"""

:param fit_function:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some parameter info please

@@ -179,6 +204,7 @@ def get_fitbenchmark_ties(param_set, ties):
else:
tie = param_set[start + 1:comma]
ties_per_function.append(tie.replace("=", "': "))
# ties_per_function.append(tie)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dead comment?

@@ -59,7 +59,7 @@ def __init__(self, fname):
#String containing the function name(s) and the starting parameter values for each function
self._equation = entries['function']

self._starting_values = None
# self._starting_values = (entries['function'].split(',', 1))[1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dead comment?

Copy link
Contributor

@Anders-Markvardsen Anders-Markvardsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functional testing all passes.
many thanks for this work - a monster PR!
Please address suggestions for changes in comments

@Anders-Markvardsen
Copy link
Contributor

All definitions works with Scipy and Mantid. All work for SasView except for FitBenchmark type problems. For example by setting problem_sets = ["Neutron"] in example_runscript_sasview gives the error

C:\Midlertiddig\fitbenchmarking\fitbenchmarking\fitting\sasview\main.py in fit(problem, data, function, minimizer, init_func_def)
    107             # The problem type is NIST
    108             #Formatting the function parameter names
--> 109             formatted_param_names = [param[0] for param in problem.starting_values]
    110
    111         param_values = function[1]

TypeError: 'NoneType' object is not iterable

@Anders-Markvardsen
Copy link
Contributor

@AtomAnu thanks for the changes and again for your work over the summer.

I have created work for follow up work that this PR has highlighted during testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request In progress
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants