Changeset f619de7 in sasmodels

Timestamp:

Apr 11, 2016 11:14:50 AM (9 years ago)

Author:

Paul Kienzle <pkienzle@…>

Branches:

master, core_shell_microgels, costrafo411, magnetic_model, release_v0.94, release_v0.95, ticket-1257-vesicle-product, ticket_1156, ticket_1265_superball, ticket_822_more_unit_tests

Children:

7ae2b7f

Parents:

9a943d0

Message:

more type hinting

Location:

sasmodels

Files:

• core.py (modified) (6 diffs)
• generate.py (modified) (24 diffs)
• kernel.py (added)
• kernelcl.py (modified) (4 diffs)
• kerneldll.py (modified) (14 diffs)
• kernelpy.py (modified) (4 diffs)
• mixture.py (modified) (8 diffs)
• model_test.py (modified) (2 diffs)
• modelinfo.py (modified) (1 diff)
• product.py (modified) (9 diffs)

sasmodels/core.py

@@ -26 +26 @@
     HAVE_OPENCL = False
 
+try:
+    from typing import List, Union, Optional, Any
+    DType = Union[None, str, np.dtype]
+    from .kernel import KernelModel
+except ImportError:
+    pass
+
+
 # TODO: refactor composite model support
 # The current load_model_info/build_model does not reuse existing model
@@ -39 +47 @@
 
 def list_models():
+    # type: () -> List[str]
     """
     Return the list of available models on the model path.
@@ -48 +57 @@
 
 def isstr(s):
+    # type: (Any) -> bool
     """
     Return True if *s* is a string-like object.
@@ -55 +65 @@
     return True
 
-def load_model(model_name, **kw):
+def load_model(model_name, dtype=None, platform='ocl'):
+    # type: (str, DType, str) -> KernelModel
     """
     Load model info and build model.
+
+    *model_name* is the name of the model as used by :func:`load_model_info`.
+    Additional keyword arguments are passed directly to :func:`build_model`.
     """
-    return build_model(load_model_info(model_name), **kw)
+    return build_model(load_model_info(model_name),
+                       dtype=dtype, platform=platform)
 
 
 def load_model_info(model_name):
+    # type: (str) -> modelinfo.ModelInfo
     """
     Load a model definition given the model name.
@@ -86 +102 @@
 
 def build_model(model_info, dtype=None, platform="ocl"):
+    # type: (modelinfo.ModelInfo, DType, str) -> KernelModel
     """
     Prepare the model for the default execution platform.
@@ -138 +155 @@
 
 def precompile_dll(model_name, dtype="double"):
+    # type: (str, DType) -> Optional[str]
     """
     Precompile the dll for a model.

sasmodels/generate.py

@@ -164 +164 @@
 from .modelinfo import Parameter
 from .custom import load_custom_kernel_module
+
+try:
+    from typing import Tuple, Sequence, Iterator
+    from .modelinfo import ModelInfo
+except ImportError:
+    pass
 
 TEMPLATE_ROOT = dirname(__file__)
@@ -220 +226 @@
 
 def format_units(units):
+    # type: (str) -> str
     """
     Convert units into ReStructured Text format.
@@ -226 +233 @@
 
 def make_partable(pars):
+    # type: (List[Parameter]) -> str
     """
     Generate the parameter table to include in the sphinx documentation.
@@ -256 +264 @@
 
 def _search(search_path, filename):
+    # type: (List[str], str) -> str
     """
     Find *filename* in *search_path*.
@@ -269 +278 @@
 
 def model_sources(model_info):
+    # type: (ModelInfo) -> List[str]
     """
     Return a list of the sources file paths for the module.
@@ -277 +287 @@
 
 def timestamp(model_info):
+    # type: (ModelInfo) -> int
     """
     Return a timestamp for the model corresponding to the most recently
@@ -288 +299 @@
 
 def convert_type(source, dtype):
+    # type: (str, np.dtype) -> str
     """
     Convert code from double precision to the desired type.
@@ -312 +324 @@
 
 def _convert_type(source, type_name, constant_flag):
+    # type: (str, str, str) -> str
     """
     Replace 'double' with *type_name* in *source*, tagging floating point
@@ -330 +343 @@
 
 def kernel_name(model_info, is_2d):
+    # type: (ModelInfo, bool) -> str
     """
     Name of the exported kernel symbol.
@@ -337 +351 @@
 
 def indent(s, depth):
+    # type: (str, int) -> str
     """
     Indent a string of text with *depth* additional spaces on each line.
@@ -345 +360 @@
 
 
-_template_cache = {}
+_template_cache = {}  # type: Dict[str, Tuple[int, str, str]]
 def load_template(filename):
+    # type: (str) -> str
     path = joinpath(TEMPLATE_ROOT, filename)
     mtime = getmtime(path)
@@ -355 +371 @@
 
 def model_templates():
+    # type: () -> List[str]
     # TODO: fails DRY; templates are listed in two places.
     # should instead have model_info contain a list of paths
@@ -371 +388 @@
 
 def _gen_fn(name, pars, body):
+    # type: (str, List[Parameter], str) -> str
     """
     Generate a function given pars and body.
@@ -385 +403 @@
 
 def _call_pars(prefix, pars):
+    # type: (str, List[Parameter]) -> List[str]
     """
     Return a list of *prefix.parameter* from parameter items.
@@ -393 +412 @@
                            flags=re.MULTILINE)
 def _have_Iqxy(sources):
+    # type: (List[str]) -> bool
     """
     Return true if any file defines Iqxy.
@@ -414 +434 @@
 
 def make_source(model_info):
+    # type: (ModelInfo) -> str
     """
     Generate the OpenCL/ctypes kernel from the module info.
 
-    Uses source files found in the given search path.
+    Uses source files found in the given search path.  Returns None if this
+    is a pure python model, with no C source components.
     """
     if callable(model_info.Iq):
-        return None
+        raise ValueError("can't compile python model")
 
     # TODO: need something other than volume to indicate dispersion parameters
@@ -447 +469 @@
     q, qx, qy = [Parameter(name=v) for v in ('q', 'qx', 'qy')]
     # Generate form_volume function, etc. from body only
-    if model_info.form_volume is not None:
+    if isinstance(model_info.form_volume, str):
         pars = partable.form_volume_parameters
         source.append(_gen_fn('form_volume', pars, model_info.form_volume))
-    if model_info.Iq is not None:
+    if isinstance(model_info.Iq, str):
         pars = [q] + partable.iq_parameters
         source.append(_gen_fn('Iq', pars, model_info.Iq))
-    if model_info.Iqxy is not None:
+    if isinstance(model_info.Iqxy, str):
         pars = [qx, qy] + partable.iqxy_parameters
         source.append(_gen_fn('Iqxy', pars, model_info.Iqxy))
@@ -509 +531 @@
 
 def load_kernel_module(model_name):
+    # type: (str) -> module
     if model_name.endswith('.py'):
         kernel_module = load_custom_kernel_module(model_name)
@@ -522 +545 @@
                             %re.escape(string.punctuation))
 def _convert_section_titles_to_boldface(lines):
+    # type: (Sequence[str]) -> Iterator[str]
     """
     Do the actual work of identifying and converting section headings.
@@ -543 +567 @@
 
 def convert_section_titles_to_boldface(s):
+    # type: (str) -> str
     """
     Use explicit bold-face rather than section headings so that the table of
@@ -553 +578 @@
 
 def make_doc(model_info):
+    # type: (ModelInfo) -> str
     """
     Return the documentation for the model.
@@ -562 +588 @@
                  name=model_info.name,
                  title=model_info.title,
-                 parameters=make_partable(model_info.parameters),
+                 parameters=make_partable(model_info.parameters.kernel_parameters),
                  returns=Sq_units if model_info.structure_factor else Iq_units,
                  docs=docs)
@@ -569 +595 @@
 
 def demo_time():
+    # type: () -> None
     """
     Show how long it takes to process a model.
@@ -582 +609 @@
 
 def main():
+    # type: () -> None
     """
     Program which prints the source produced by the model.

sasmodels/kernelcl.py

@@ -67 +67 @@
 
 from . import generate
+from .kernel import KernelModel, Kernel
 
 # The max loops number is limited by the amount of local memory available
@@ -310 +311 @@
 
 
-class GpuModel(object):
+class GpuModel(KernelModel):
     """
     GPU wrapper for a single model.
@@ -420 +421 @@
         self.release()
 
-class GpuKernel(object):
+class GpuKernel(Kernel):
     """
     Callable SAS kernel.
@@ -489 +490 @@
         self.kernel(self.queue, self.q_input.global_size, None, *args)
         cl.enqueue_copy(self.queue, self.result, self.result_b)
-        [v.release() for v in details_b, weights_b, values_b]
+        [v.release() for v in (details_b, weights_b, values_b)]
 
         return self.result[:self.nq]

sasmodels/kerneldll.py

@@ -56 +56 @@
 from . import generate
 from . import details
-from .kernelpy import PyInput, PyModel
+from .kernel import KernelModel, Kernel
+from .kernelpy import PyInput
 from .exception import annotate_exception
+from .generate import F16, F32, F64
+
+try:
+    from typing import Tuple, Callable, Any
+    from .modelinfo import ModelInfo
+    from .details import CallDetails
+except ImportError:
+    pass
 
 # Compiler platform details
@@ -91 +100 @@
 
 def dll_name(model_info, dtype):
+    # type: (ModelInfo, np.dtype) ->  str
     """
     Name of the dll containing the model.  This is the base file name without
@@ -98 +108 @@
     return "sas_%s%d"%(model_info.id, bits)
 
+
 def dll_path(model_info, dtype):
+    # type: (ModelInfo, np.dtype) -> str
     """
     Complete path to the dll for the model.  Note that the dll may not
@@ -105 +117 @@
     return os.path.join(DLL_PATH, dll_name(model_info, dtype)+".so")
 
-def make_dll(source, model_info, dtype="double"):
-    """
-    Load the compiled model defined by *kernel_module*.
-
-    Recompile if any files are newer than the model file.
+
+def make_dll(source, model_info, dtype=F64):
+    # type: (str, ModelInfo, np.dtype) -> str
+    """
+    Returns the path to the compiled model defined by *kernel_module*.
+
+    If the model has not been compiled, or if the source file(s) are newer
+    than the dll, then *make_dll* will compile the model before returning.
+    This routine does not load the resulting dll.
 
     *dtype* is a numpy floating point precision specifier indicating whether
-    the model should be single or double precision.  The default is double
-    precision.
-
-    The DLL is not loaded until the kernel is called so models can
-    be defined without using too many resources.
+    the model should be single, double or long double precision.  The default
+    is double precision, *np.dtype('d')*.
+
+    Set *sasmodels.ALLOW_SINGLE_PRECISION_DLLS* to False if single precision
+    models are not allowed as DLLs.
 
     Set *sasmodels.kerneldll.DLL_PATH* to the compiled dll output path.
     The default is the system temporary directory.
-
-    Set *sasmodels.ALLOW_SINGLE_PRECISION_DLLS* to True if single precision
-    models are allowed as DLLs.
-    """
-    if callable(model_info.Iq):
-        return PyModel(model_info)
-    
-    dtype = np.dtype(dtype)
-    if dtype == generate.F16:
+    """
+    if dtype == F16:
         raise ValueError("16 bit floats not supported")
-    if dtype == generate.F32 and not ALLOW_SINGLE_PRECISION_DLLS:
-        dtype = generate.F64  # Force 64-bit dll
-
-    source = generate.convert_type(source, dtype)
+    if dtype == F32 and not ALLOW_SINGLE_PRECISION_DLLS:
+        dtype = F64  # Force 64-bit dll
+    # Note: dtype may be F128 for long double precision
+
     newest = generate.timestamp(model_info)
     dll = dll_path(model_info, dtype)
@@ -139 +148 @@
         basename = dll_name(model_info, dtype) + "_"
         fid, filename = tempfile.mkstemp(suffix=".c", prefix=basename)
+        source = generate.convert_type(source, dtype)
         os.fdopen(fid, "w").write(source)
         command = COMPILE%{"source":filename, "output":dll}
@@ -152 +162 @@
 
 
-def load_dll(source, model_info, dtype="double"):
+def load_dll(source, model_info, dtype=F64):
+    # type: (str, ModelInfo, np.dtype) -> "DllModel"
     """
     Create and load a dll corresponding to the source, info pair returned
@@ -163 +174 @@
     return DllModel(filename, model_info, dtype=dtype)
 
-class DllModel(object):
+
+class DllModel(KernelModel):
     """
     ctypes wrapper for a single model.
@@ -179 +191 @@
     
     def __init__(self, dllpath, model_info, dtype=generate.F32):
+        # type: (str, ModelInfo, np.dtype) -> None
         self.info = model_info
         self.dllpath = dllpath
-        self.dll = None
+        self._dll = None  # type: ct.CDLL
         self.dtype = np.dtype(dtype)
 
     def _load_dll(self):
+        # type: () -> None
         #print("dll", self.dllpath)
         try:
-            self.dll = ct.CDLL(self.dllpath)
+            self._dll = ct.CDLL(self.dllpath)
         except:
             annotate_exception("while loading "+self.dllpath)
@@ -198 +212 @@
         # int, int, int, int*, double*, double*, double*, double*, double*, double
         argtypes = [c_int32]*3 + [c_void_p]*5 + [fp]
-        self.Iq = self.dll[generate.kernel_name(self.info, False)]
-        self.Iqxy = self.dll[generate.kernel_name(self.info, True)]
-        self.Iq.argtypes = argtypes
-        self.Iqxy.argtypes = argtypes
+        self._Iq = self._dll[generate.kernel_name(self.info, is_2d=False)]
+        self._Iqxy = self._dll[generate.kernel_name(self.info, is_2d=True)]
+        self._Iq.argtypes = argtypes
+        self._Iqxy.argtypes = argtypes
 
     def __getstate__(self):
+        # type: () -> Tuple[ModelInfo, str]
         return self.info, self.dllpath
 
     def __setstate__(self, state):
+        # type: (Tuple[ModelInfo, str]) -> None
         self.info, self.dllpath = state
-        self.dll = None
+        self._dll = None
 
     def make_kernel(self, q_vectors):
+        # type: (List[np.ndarray]) -> DllKernel
         q_input = PyInput(q_vectors, self.dtype)
-        if self.dll is None: self._load_dll()
-        kernel = self.Iqxy if q_input.is_2d else self.Iq
+        # Note: pickle not supported for DllKernel
+        if self._dll is None:
+            self._load_dll()
+        kernel = self._Iqxy if q_input.is_2d else self._Iq
         return DllKernel(kernel, self.info, q_input)
 
     def release(self):
+        # type: () -> None
         """
         Release any resources associated with the model.
@@ -225 +245 @@
             libHandle = dll._handle
             #libHandle = ct.c_void_p(dll._handle)
-            del dll, self.dll
-            self.dll = None
+            del dll, self._dll
+            self._dll = None
             #_ctypes.FreeLibrary(libHandle)
             ct.windll.kernel32.FreeLibrary(libHandle)
@@ -233 +253 @@
 
 
-class DllKernel(object):
+class DllKernel(Kernel):
     """
     Callable SAS kernel.
@@ -253 +273 @@
     """
     def __init__(self, kernel, model_info, q_input):
+        # type: (Callable[[], np.ndarray], ModelInfo, PyInput) -> None
         self.kernel = kernel
         self.info = model_info
@@ -261 +282 @@
 
     def __call__(self, call_details, weights, values, cutoff):
+        # type: (CallDetails, np.ndarray, np.ndarray, float) -> np.ndarray
         real = (np.float32 if self.q_input.dtype == generate.F32
                 else np.float64 if self.q_input.dtype == generate.F64
@@ -282 +304 @@
             real(cutoff), # cutoff
             ]
-        self.kernel(*args)
+        self.kernel(*args) # type: ignore
         return self.result[:-3]
 
     def release(self):
+        # type: () -> None
         """
         Release any resources associated with the kernel.
         """
-        pass
+        self.q_input.release()

sasmodels/kernelpy.py

@@ -12 +12 @@
 from . import details
 from .generate import F64
+from .kernel import KernelModel, Kernel
 
 try:
@@ -20 +21 @@
     DType = Union[None, str, np.dtype]
 
-class PyModel(object):
+class PyModel(KernelModel):
     """
     Wrapper for pure python models.
@@ -77 +78 @@
         self.q = None
 
-class PyKernel(object):
+class PyKernel(Kernel):
     """
     Callable SAS kernel.
@@ -162 +163 @@
         Free resources associated with the kernel.
         """
+        self.q_input.release()
         self.q_input = None
 

sasmodels/mixture.py

@@ -15 +15 @@
 
 from .modelinfo import Parameter, ParameterTable, ModelInfo
+from .kernel import KernelModel, Kernel
+
+try:
+    from typing import List
+    from .details import CallDetails
+except ImportError:
+    pass
 
 def make_mixture_info(parts):
+    # type: (List[ModelInfo]) -> ModelInfo
     """
     Create info block for product model.
@@ -22 +30 @@
     flatten = []
     for part in parts:
-        if part['composition'] and part['composition'][0] == 'mixture':
-            flatten.extend(part['compostion'][1])
+        if part.composition and part.composition[0] == 'mixture':
+            flatten.extend(part.composition[1])
         else:
             flatten.append(part)
@@ -29 +37 @@
 
     # Build new parameter list
-    pars = []
+    combined_pars = []
     for k, part in enumerate(parts):
         # Parameter prefix per model, A_, B_, ...
@@ -35 +43 @@
         # to support vector parameters
         prefix = chr(ord('A')+k) + '_'
-        pars.append(Parameter(prefix+'scale'))
-        for p in part['parameters'].kernel_pars:
+        combined_pars.append(Parameter(prefix+'scale'))
+        for p in part.parameters.kernel_parameters:
             p = copy(p)
-            p.name = prefix+p.name
-            p.id = prefix+p.id
+            p.name = prefix + p.name
+            p.id = prefix + p.id
             if p.length_control is not None:
-                p.length_control = prefix+p.length_control
-            pars.append(p)
-    partable = ParameterTable(pars)
+                p.length_control = prefix + p.length_control
+            combined_pars.append(p)
+    parameters = ParameterTable(combined_pars)
 
     model_info = ModelInfo()
-    model_info.id = '+'.join(part['id'])
-    model_info.name = ' + '.join(part['name'])
+    model_info.id = '+'.join(part.id for part in parts)
+    model_info.name = ' + '.join(part.name for part in parts)
     model_info.filename = None
     model_info.title = 'Mixture model with ' + model_info.name
@@ -53 +61 @@
     model_info.docs = model_info.title
     model_info.category = "custom"
-    model_info.parameters = partable
+    model_info.parameters = parameters
     #model_info.single = any(part['single'] for part in parts)
     model_info.structure_factor = False
@@ -64 +72 @@
 
 
-class MixtureModel(object):
+class MixtureModel(KernelModel):
     def __init__(self, model_info, parts):
+        # type: (ModelInfo, List[KernelModel]) -> None
         self.info = model_info
         self.parts = parts
 
     def __call__(self, q_vectors):
+        # type: (List[np.ndarray]) -> MixtureKernel
         # Note: may be sending the q_vectors to the n times even though they
         # are only needed once.  It would mess up modularity quite a bit to
@@ -76 +86 @@
         # in opencl; or both in opencl, but one in single precision and the
         # other in double precision).
-        kernels = [part(q_vectors) for part in self.parts]
+        kernels = [part.make_kernel(q_vectors) for part in self.parts]
         return MixtureKernel(self.info, kernels)
 
     def release(self):
+        # type: () -> None
         """
         Free resources associated with the model.
@@ -87 +98 @@
 
 
-class MixtureKernel(object):
+class MixtureKernel(Kernel):
     def __init__(self, model_info, kernels):
-        dim = '2d' if kernels[0].q_input.is_2d else '1d'
+        # type: (ModelInfo, List[Kernel]) -> None
+        self.dim = kernels[0].dim
+        self.info =  model_info
+        self.kernels = kernels
 
-        # fixed offsets starts at 2 for scale and background
-        fixed_pars, pd_pars = [], []
-        offsets = [[2, 0]]
-        #vol_index = []
-        def accumulate(fixed, pd, volume):
-            # subtract 1 from fixed since we are removing background
-            fixed_offset, pd_offset = offsets[-1]
-            #vol_index.extend(k+pd_offset for k,v in pd if v in volume)
-            offsets.append([fixed_offset + len(fixed) - 1, pd_offset + len(pd)])
-            pd_pars.append(pd)
-        if dim == '2d':
-            for p in kernels:
-                partype = p.info.partype
-                accumulate(partype['fixed-2d'], partype['pd-2d'], partype['volume'])
-        else:
-            for p in kernels:
-                partype = p.info.partype
-                accumulate(partype['fixed-1d'], partype['pd-1d'], partype['volume'])
-
-        #self.vol_index = vol_index
-        self.offsets = offsets
-        self.fixed_pars = fixed_pars
-        self.pd_pars = pd_pars
-        self.info = model_info
-        self.kernels = kernels
-        self.results = None
-
-    def __call__(self, fixed_pars, pd_pars, cutoff=1e-5):
-        scale, background = fixed_pars[0:2]
+    def __call__(self, call_details, value, weight, cutoff):
+        # type: (CallDetails, np.ndarray, np.ndarry, float) -> np.ndarray
+        scale, background = value[0:2]
         total = 0.0
-        self.results = []  # remember the parts for plotting later
-        for k in range(len(self.offsets)-1):
-            start_fixed, start_pd = self.offsets[k]
-            end_fixed, end_pd = self.offsets[k+1]
-            part_fixed = [fixed_pars[start_fixed], 0.0] + fixed_pars[start_fixed+1:end_fixed]
-            part_pd = [pd_pars[start_pd], 0.0] + pd_pars[start_pd+1:end_pd]
-            part_result = self.kernels[k](part_fixed, part_pd)
+        # remember the parts for plotting later
+        self.results = []
+        for kernel, kernel_details in zip(self.kernels, call_details.parts):
+            part_result = kernel(kernel_details, value, weight, cutoff)
             total += part_result
-            self.results.append(scale*sum+background)
+            self.results.append(part_result)
 
         return scale*total + background
 
     def release(self):
-        self.p_kernel.release()
-        self.q_kernel.release()
+        # type: () -> None
+        for k in self.kernels:
+            k.release()
 

sasmodels/model_test.py

@@ -69 +69 @@
     # type: (ModelInfo, ParameterSet) -> float
     """
-    Call the model ER function using *values*. *model_info* is either
-    *model.info* if you have a loaded model, or *kernel.info* if you
-    have a model kernel prepared for evaluation.
+    Call the model ER function using *values*.
+
+    *model_info* is either *model.info* if you have a loaded model,
+    or *kernel.info* if you have a model kernel prepared for evaluation.
     """
     if model_info.ER is None:
@@ -84 +85 @@
     """
     Call the model VR function using *pars*.
-    *info* is either *model.info* if you have a loaded model, or *kernel.info*
-    if you have a model kernel prepared for evaluation.
+
+    *model_info* is either *model.info* if you have a loaded model,
+    or *kernel.info* if you have a model kernel prepared for evaluation.
     """
     if model_info.VR is None:

sasmodels/modelinfo.py

@@ -713 +713 @@
     ER = None               # type: Optional[Callable[[np.ndarray], np.ndarray]]
     VR = None               # type: Optional[Callable[[np.ndarray], Tuple[np.ndarray, np.ndarray]]]
-    form_volume = None      # type: Optional[Callable[[np.ndarray], float]]
-    Iq = None               # type: Optional[Callable[[np.ndarray], np.ndarray]]
-    Iqxy = None             # type: Optional[Callable[[np.ndarray], np.ndarray]]
+    form_volume = None      # type: Union[None, str, Callable[[np.ndarray], float]]
+    Iq = None               # type: Union[None, str, Callable[[np.ndarray], np.ndarray]]
+    Iqxy = None             # type: Union[None, str, Callable[[np.ndarray], np.ndarray]]
     profile = None          # type: Optional[Callable[[np.ndarray], None]]
     sesans = None           # type: Optional[Callable[[np.ndarray], np.ndarray]]

sasmodels/product.py

@@ -14 +14 @@
 
 from .details import dispersion_mesh
-from .modelinfo import suffix_parameter, ParameterTable, Parameter, ModelInfo
+from .modelinfo import suffix_parameter, ParameterTable, ModelInfo
+from .kernel import KernelModel, Kernel
+
+try:
+    from typing import Tuple
+    from .modelinfo import ParameterSet
+    from .details import CallDetails
+except ImportError:
+    pass
 
 # TODO: make estimates available to constraints
@@ -25 +33 @@
 # revert it after making VR and ER available at run time as constraints.
 def make_product_info(p_info, s_info):
+    # type: (ModelInfo, ModelInfo) -> ModelInfo
     """
     Create info block for product model.
     """
-    p_id, p_name, p_partable = p_info.id, p_info.name, p_info.parameters
-    s_id, s_name, s_partable = s_info.id, s_info.name, s_info.parameters
-    p_set = set(p.id for p in p_partable)
-    s_set = set(p.id for p in s_partable)
+    p_id, p_name, p_pars = p_info.id, p_info.name, p_info.parameters
+    s_id, s_name, s_pars = s_info.id, s_info.name, s_info.parameters
+    p_set = set(p.id for p in p_pars.call_parameters)
+    s_set = set(p.id for p in s_pars.call_parameters)
 
     if p_set & s_set:
         # there is some overlap between the parameter names; tag the
         # overlapping S parameters with name_S
-        s_pars = [(suffix_parameter(par, "_S") if par.id in p_set else par)
-                  for par in s_partable.kernel_parameters]
-        pars = p_partable.kernel_parameters + s_pars
+        s_list = [(suffix_parameter(par, "_S") if par.id in p_set else par)
+                  for par in s_pars.kernel_parameters]
+        combined_pars = p_pars.kernel_parameters + s_list
     else:
-        pars= p_partable.kernel_parameters + s_partable.kernel_parameters
+        combined_pars = p_pars.kernel_parameters + s_pars.kernel_parameters
+    parameters = ParameterTable(combined_pars)
 
     model_info = ModelInfo()
@@ -50 +60 @@
     model_info.docs = model_info.title
     model_info.category = "custom"
-    model_info.parameters = ParameterTable(pars)
+    model_info.parameters = parameters
     #model_info.single = p_info.single and s_info.single
     model_info.structure_factor = False
@@ -60 +70 @@
     return model_info
 
-class ProductModel(object):
+class ProductModel(KernelModel):
     def __init__(self, model_info, P, S):
+        # type: (ModelInfo, KernelModel, KernelModel) -> None
         self.info = model_info
         self.P = P
@@ -67 +78 @@
 
     def __call__(self, q_vectors):
+        # type: (List[np.ndarray]) -> Kernel
         # Note: may be sending the q_vectors to the GPU twice even though they
         # are only needed once.  It would mess up modularity quite a bit to
@@ -73 +85 @@
         # in opencl; or both in opencl, but one in single precision and the
         # other in double precision).
-        p_kernel = self.P(q_vectors)
-        s_kernel = self.S(q_vectors)
+        p_kernel = self.P.make_kernel(q_vectors)
+        s_kernel = self.S.make_kernel(q_vectors)
         return ProductKernel(self.info, p_kernel, s_kernel)
 
     def release(self):
+        # type: (None) -> None
         """
         Free resources associated with the model.
@@ -85 +98 @@
 
 
-class ProductKernel(object):
+class ProductKernel(Kernel):
     def __init__(self, model_info, p_kernel, s_kernel):
+        # type: (ModelInfo, Kernel, Kernel) -> None
         self.info = model_info
         self.p_kernel = p_kernel
@@ -92 +106 @@
 
     def __call__(self, details, weights, values, cutoff):
+        # type: (CallDetails, np.ndarray, np.ndarray, float) -> np.ndarray
         effect_radius, vol_ratio = call_ER_VR(self.p_kernel.info, vol_pars)
 
@@ -108 +123 @@
 
     def release(self):
+        # type: () -> None
         self.p_kernel.release()
-        self.q_kernel.release()
+        self.s_kernel.release()
 
-def call_ER_VR(model_info, vol_pars):
+def call_ER_VR(model_info, pars):
     """
     Return effect radius and volume ratio for the model.
     """
-    value, weight = dispersion_mesh(vol_pars)
+    if model_info.ER is None and model_info.VR is None:
+        return 1.0, 1.0
 
-    individual_radii = model_info.ER(*value) if model_info.ER else 1.0
-    whole, part = model_info.VR(*value) if model_info.VR else (1.0, 1.0)
+    value, weight = _vol_pars(model_info, pars)
 
-    effect_radius = np.sum(weight*individual_radii) / np.sum(weight)
-    volume_ratio = np.sum(weight*part)/np.sum(weight*whole)
+    if model_info.ER is not None:
+        individual_radii = model_info.ER(*value)
+        effect_radius = np.sum(weight*individual_radii) / np.sum(weight)
+    else:
+        effect_radius = 1.0
+
+    if model_info.VR is not None:
+        whole, part = model_info.VR(*value)
+        volume_ratio = np.sum(weight*part)/np.sum(weight*whole)
+    else:
+        volume_ratio = 1.0
+
     return effect_radius, volume_ratio
+
+def _vol_pars(model_info, pars):
+    # type: (ModelInfo, ParameterSet) -> Tuple[np.ndarray, np.ndarray]
+    vol_pars = [get_weights(p, pars)
+                for p in model_info.parameters.call_parameters
+                if p.type == 'volume']
+    value, weight = dispersion_mesh(model_info, vol_pars)
+    return value, weight
+