diff --git a/docs/source/diffpy-agent.rst b/docs/source/diffpy-agent.rst index 3acde05..dc8eb01 100644 --- a/docs/source/diffpy-agent.rst +++ b/docs/source/diffpy-agent.rst @@ -18,6 +18,7 @@ Setup ----- .. _connect_claude_desktop: + Connect to Claude Desktop ~~~~~~~~~~~~~~~~~~~~~~~~~ 1. Copy the path to the conda executable. There are many commands available, @@ -52,7 +53,7 @@ here are two examples "", "python", "-m", - "diffpy.apps.refinebase.refinement_server" + "diffpy.apps.parametric_refinement.refinement_server" ] } } @@ -74,7 +75,7 @@ would look like this: "diffpy_apps", "python", "-m", - "diffpy.apps.refinebase.refinement_server" + "diffpy.apps.parametric_refinement.refinement_server" ] } }, @@ -96,7 +97,7 @@ would look like this: "diffpy_apps", "python", "-m", - "diffpy.apps.refinebase.refinement_server" + "diffpy.apps.parametric_refinement.refinement_server" ] } }, @@ -117,6 +118,7 @@ would look like this: .. _connect_chatgpt_desktop: + Connect to ChatGPT Desktop ~~~~~~~~~~~~~~~~~~~~~~~~~~ 1. Copy the path to the conda executable. There are many commands available, @@ -142,7 +144,7 @@ here are two examples "", "python", "-m", - "diffpy.apps.refinebase.refinement_server" + "diffpy.apps.parametric_refinement.refinement_server" ] .. note:: diff --git a/news/patch.rst b/news/patch.rst new file mode 100644 index 0000000..b847fcd --- /dev/null +++ b/news/patch.rst @@ -0,0 +1,23 @@ +**Added:** + +* No news added: Small internal code refactor and workflow adjustment. + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* diff --git a/requirements/tests.txt b/requirements/tests.txt index a727786..d9d0c8c 100644 --- a/requirements/tests.txt +++ b/requirements/tests.txt @@ -4,3 +4,4 @@ codecov coverage pytest-cov pytest-env +pytest-mock diff --git a/src/diffpy/apps/app_agentify.py b/src/diffpy/apps/app_agentify.py index d9e1fc7..a117940 100644 --- a/src/diffpy/apps/app_agentify.py +++ b/src/diffpy/apps/app_agentify.py @@ -1,3 +1,4 @@ +import json import shutil import socket import subprocess @@ -44,9 +45,12 @@ def agentify(args): ["git", "clone", REPO_URL, str(tmp_path)], check=True, ) + skills = json.load(open(tmp_path / "skills.json")) for item in tmp_path.iterdir(): if item.is_dir(): destination = dest_dir / item.name + if item.name not in skills: + continue if destination.exists(): if args.update: shutil.rmtree(destination) diff --git a/src/diffpy/apps/refinebase/__init__.py b/src/diffpy/apps/parametric_refinement/__init__.py similarity index 100% rename from src/diffpy/apps/refinebase/__init__.py rename to src/diffpy/apps/parametric_refinement/__init__.py diff --git a/src/diffpy/apps/refinebase/parametric_model.py b/src/diffpy/apps/parametric_refinement/parametric_model.py similarity index 66% rename from src/diffpy/apps/refinebase/parametric_model.py rename to src/diffpy/apps/parametric_refinement/parametric_model.py index ca8ddd3..211ebfc 100644 --- a/src/diffpy/apps/refinebase/parametric_model.py +++ b/src/diffpy/apps/parametric_refinement/parametric_model.py @@ -1,11 +1,14 @@ import logging import re +from abc import ABC, abstractmethod from functools import wraps from pathlib import Path import networkx as nx import numpy from pyobjcryst import loadCrystal +from pyobjcryst.crystal import Crystal +from pyobjcryst.molecule import Molecule from diffpy.srfit.fitbase import FitContribution from diffpy.srfit.fitbase.parameter import Parameter, ParameterProxy @@ -18,27 +21,63 @@ ObjCrystMolAtomParSet, ObjCrystMoleculeParSet, ) +from diffpy.srfit.structure.srrealparset import SrRealParSet +from diffpy.structure import Structure from diffpy.structure.parsers import get_parser # NOTE: MCP server prefers logging for output logger = logging.getLogger(__name__) -class ParametricModel: +class ParametricModelABC(ABC): + """Abstract base class for parametric models.""" + + @abstractmethod + def set_profile(self, profile, **kwargs): + pass + + @property + @abstractmethod + def operator(self): + pass + + @abstractmethod + def evaluate(self): + pass + + @abstractmethod + def residual(self): + pass + + +class ParametricModelBase: + """Base class for parametric models providing common functionality + for managing parameters and constructing a parameter graph. + + Attributes + ---------- + name : str + The name of the parametric model. + calc_obj : object + The calculation object associated with the model. + parset_obj : diffpy.srfit.fitbase.recipeorganizer.RecipeOrganizer + The parameter set object associated with the model. + _graph : networkx.DiGraph + The directed graph representing the parameter hierarchy. + """ + def __init__(self, name): self.name = name - self._contribution = FitContribution(name) - self.calc_obj = self._contribution + self.calc_obj = None + self.parset_obj = None self._graph = nx.DiGraph() - # all submodels will share the same profile - self._submodels = [] def _construct_parameter_graph( - self, parameterset, prefix="", old_graph=None + self, parset_obj, prefix="", old_graph=None ): - parent_name = f"{prefix}{parameterset.name}" - self._graph.add_node(parent_name, obj=parameterset) - for par in parameterset._iter_local_parameters(regexp=re.compile("")): + parent_name = f"{prefix}{parset_obj.name}" + self._graph.add_node(parent_name, obj=parset_obj) + for par in parset_obj._iter_local_parameters(regexp=re.compile("")): par_node_id = f"{parent_name}.{par.name}" if not old_graph or par_node_id not in old_graph.nodes: self._graph.add_node( @@ -55,7 +94,7 @@ def _construct_parameter_graph( ], ) self._graph.add_edge(parent_name, par_node_id) - for obj in parameterset._iter_managed_parameter_containers(): + for obj in parset_obj._iter_managed_parameter_containers(): if hasattr(obj, "_iter_managed_parameter_containers"): child_name = f"{parent_name}.{obj.name}" # obj is handled as unconstrained by default @@ -69,55 +108,6 @@ def _construct_parameter_graph( obj, prefix=f"{parent_name}.", old_graph=old_graph ) - def register_submodel(self, submodel, symbol=None): - if not isinstance(self, ParametricModelEquation): - raise ValueError( - "Submodels can only be registered to " - "ParametricModelEquation instance." - ) - if symbol is None: - symbol = submodel.name - if symbol in self._contribution._parameters: - self._contribution._remove_parameter( - self._contribution._parameters[symbol] - ) - if isinstance(submodel, ParametricModelPDF): - if symbol != submodel.name: - logger.warning( - f"ParametricModelPDF's name ({submodel.name}) does " - f"not match with the provided symbol ({symbol}) ", - ) - self._contribution.add_profile_generator(submodel.calc_obj) - elif isinstance(submodel, ParametricModelEquation) or isinstance( - submodel, ParametricModelFunction - ): - self._contribution._eqfactory.registerOperator( - symbol, submodel._contribution._eq - ) - self._contribution.add_parameter_set(submodel._contribution) - else: - raise NotImplementedError( - "Only ParametricModelPDF, ParametricModelEquation, " - "and ParametricModelFunction " - "instances are supported to be registered as submodels." - ) - if self.equation_str is not None: - self._contribution.set_equation(self.equation_str) - submodel._rebuild_graph() - if f"{self.name}.{submodel.name}" not in self._graph.nodes: - self._graph.add_node( - symbol, - obj=submodel, - constrained_or_constant=False, - ) - self._graph.add_edge(self.name, symbol) - subgraph = submodel._graph.copy() - mapping = {node: f"{self.name}.{node}" for node in subgraph.nodes} - subgraph = nx.relabel_nodes(subgraph, mapping) - self._graph = nx.compose(self._graph, subgraph) - self._submodels.append(submodel) - self._rebuild_graph() - @property def parameters(self): return { @@ -144,82 +134,154 @@ def independent_parameters(self): ) } - def set_profile(self, profile): - self._contribution.set_profile(profile) - for submodel in self._submodels: - if hasattr(submodel, "set_profile"): - submodel.set_profile(profile) - self._rebuild_graph() - def _rebuild_graph(self): old_graph = self._graph self._graph = nx.DiGraph() self._construct_parameter_graph( - self._contribution, prefix="", old_graph=old_graph + self.parset_obj, prefix="", old_graph=old_graph ) - def evaluate(self): - raise NotImplementedError( - "The evaluate method must be implemented by subclasses." - ) - def residual(self): - raise NotImplementedError( - "The residual method must be implemented by subclasses." - ) +ParametricModelABC.register(ParametricModelBase) + +class ParametricModelEquation(ParametricModelBase, ParametricModelABC): + """Parametric model based on an equation. -class ParametricModelEquation(ParametricModel): - def __init__(self, name, equation_str=None): + Attributes + ---------- + _contribution : diffpy.srfit.fitbase.FitContribution + The fit contribution object associated with the model. + calc_obj : diffpy.srfit.fitbase.FitContribution + Alias for the fit contribution object. + parset_obj : diffpy.srfit.fitbase.FitContribution + Alias for the fit contribution object. + _submodels : list + List of registered submodels. + """ + + def __init__(self, name, equation_string): super().__init__(name=name) - self.equation_str = None - if equation_str: - self.set_equation(equation_str) + self._contribution = FitContribution(name) + self.calc_obj = self._contribution + self.parset_obj = self._contribution + self._submodels = [] + self.set_equation(equation_string) - def set_equation(self, equation_str): - self.equation_str = equation_str - self._contribution.set_equation(equation_str) + def set_equation(self, equation_string): + self.equation_string = equation_string + self._contribution.set_equation(equation_string) self._rebuild_graph() - def get_equation(self): - return self.equation_str + @property + def operator(self): + return self._contribution._eq + + def set_profile(self, profile, xname=None, yname=None, dyname=None): + self._contribution.set_profile( + profile, xname=xname, yname=yname, dyname=dyname + ) + for submodel in self._submodels: + if hasattr(submodel, "set_profile"): + submodel.set_profile( + profile, xname=xname, yname=yname, dyname=dyname + ) def evaluate(self): - yc = self._contribution._eq() if ( hasattr(self._contribution, "profile") and self._contribution.profile is not None ): + yc = self._contribution._eq() self._contribution.profile.ycalc = yc + return yc + yc = self._contribution._eq() return yc def residual(self): return self._contribution.residual() + def register_submodel(self, submodel, symbol=None): + if symbol is None: + symbol = submodel.name + # add submodule's calculation model + if symbol in self._contribution._parameters: + self._contribution._remove_parameter( + self._contribution._parameters[symbol] + ) + if isinstance(submodel, ParametricModelPDF): + if symbol != submodel.name: + logger.warning( + f"ParametricModelPDF's name ({submodel.name}) does " + f"not match with the provided symbol ({symbol}). " + f"{submodel.name} will be used as the symbol." + ) + self._contribution.add_profile_generator(submodel._generator) + elif isinstance(submodel, ParametricModelFunction) or isinstance( + submodel, ParametricModelEquation + ): + self._contribution._eqfactory.registerOperator( + symbol, submodel.operator + ) + self._contribution.add_parameter_set(submodel.parset_obj) + else: + raise NotImplementedError( + "Only ParametricModelPDF, ParametricModelEquation, " + "and ParametricModelFunction " + "instances are supported to be registered as submodels." + ) + self.set_equation(self.equation_string) + # add submodel's subgraph + submodel._rebuild_graph() + if f"{self.name}.{submodel.name}" not in self._graph.nodes: + self._graph.add_node( + symbol, + obj=submodel, + constrained_or_constant=False, + ) + self._graph.add_edge(self.name, symbol) + subgraph = submodel._graph.copy() + mapping = {node: f"{self.name}.{node}" for node in subgraph.nodes} + subgraph = nx.relabel_nodes(subgraph, mapping) + self._graph = nx.compose(self._graph, subgraph) + self._submodels.append(submodel) + self._rebuild_graph() + -class ParametricModelFunction(ParametricModel): - def __init__(self, name, function, argnames=None): +class ParametricModelFunction(ParametricModelEquation): + """Parametric model based on a function.""" + + def __init__(self, name, function, symbol=None, argnames=None): """Initialize a ParametricModelFunction instance. function can be either a callable or a string representing the pre-defined function. One and only one of func or characteristic_func_name must be provided. - Allowed value for characteristic_func_name: - "spherical_particle", - "spheroidal_particle", - "lognormal_spherical_particle", - "sheet_particle", - "shell_particle", - "SASCF", - "sphericalCF", - "spheroidalCF", - "spheroidalCF2", - "lognormalSphericalCF", - "sheetCF", - "shellCF", - "shellCF2", + + Parameters + ---------- + name: str + The name of the parametric model function. + symbol: str + The symbol representing the function in the equation. + function: str or callable + The function associated with the model. Allowed str values are: + "spherical_particle", + "spheroidal_particle", + "lognormal_spherical_particle", + "sheet_particle", + "shell_particle", + "SASCF", + "sphericalCF", + "spheroidalCF", + "spheroidalCF2", + "lognormalSphericalCF", + "sheetCF", + "shellCF", + "shellCF2", + argnames: list of str, optional + The names of the arguments for the function. """ - super().__init__(name=name) if isinstance(function, str): import diffpy.srfit.pdf.characteristicfunctions @@ -227,30 +289,49 @@ def __init__(self, name, function, argnames=None): diffpy.srfit.pdf.characteristicfunctions, function, ) - self._contribution.register_function(function, argnames=argnames) - self._contribution.set_equation(function.__name__) - self.calc_obj = function - self._rebuild_graph() - - def set_profile(self, profile, xname=None, yname=None, dyname=None): - self._contribution.set_profile( - profile, xname=xname, yname=yname, dyname=dyname + if symbol is None: + symbol = function.__name__ + ParametricModelBase.__init__(self, name=name) + self._contribution = FitContribution(name) + self.calc_obj = self._contribution + self.parset_obj = self._contribution + self._submodels = [] + self._function = function + self._argnames = argnames + self._contribution.register_function( + function=function, name=symbol, argnames=argnames ) - # no submodel is allowed ParametricModelFunction + self.set_equation(symbol) + self._rebuild_graph() - def evaluate(self): - return self._contribution._eq() +class ParametricModelPDF(ParametricModelBase, ParametricModelABC): + """Parametric model based on a calculator for generating pair + distribution function (PDF). + + NOTE: qmin, qmax, stype(scattering type) are meta handled throughout + the loaded profile in the refinement session + + Attributes + ---------- + _generator : PDFGenerator or DebyePDFGenerator + The generator object used internally for PDF calculations. + calc_obj: PDFGenerator or DebyePDFGenerator + alias for _generator. + parset_obj: FitContribution + The parameter set object associated with the model. + space_group_symbol: str + The space group symbol of the structure. + sgpar_names: list of str + The names of the space group parameters. + """ -class ParametricModelPDF(ParametricModel): - # NOTE: qmin, qmax, stype(scattering type) are meta handled - # throughout the loaded profile in the refinement session def __init__( self, name, structure, + finite, spacegroup_symbol="P1", - finite=False, run_parallel=True, ): """Create a ParametricModelPDF instance from a structure object. @@ -259,44 +340,68 @@ def __init__( an existing DiffpyStructureParSet/ObjCrystCrystalParSet phase (e.g. shared from another ParametricModelPDF). """ - super().__init__(name=name) + ParametricModelBase.__init__(self, name=name) if not finite: - self.calc_obj = PDFGenerator(name) - else: - self.calc_obj = DebyePDFGenerator(name) - if isinstance( - structure, (DiffpyStructureParSet, ObjCrystCrystalParSet) - ): - # already a phase parset: share it instead of rewrapping it - self.calc_obj.setPhase(structure) + self._generator = PDFGenerator(name) else: - self.calc_obj.setStructure(structure) + self._generator = DebyePDFGenerator(name) + if isinstance(structure, SrRealParSet): + self._generator.setPhase(structure) + elif isinstance(structure, (Structure, Crystal, Molecule)): + self._generator.setStructure(structure) + self.calc_obj = self._generator + self.parset_obj = self._generator + self._contribution = self.calc_obj + + self._set_parallel(run_parallel) self.space_group_symbol = spacegroup_symbol self.sgpar_names = [] + self.free_sgpar_names = [] + if isinstance(self._generator.phase, DiffpyStructureParSet): + self.use_uiso = True + elif isinstance(self._generator.phase, ObjCrystCrystalParSet): + self.use_uiso = False + else: + logger.warning( + "Unknown phase type for the generator. " + "use_uiso is set to False by default." + ) + self.use_uiso = False self._rebuild_graph() - if run_parallel: - try: - import multiprocessing - from multiprocessing import Pool - import psutil - except ImportError: - print( - "\nYou don't appear to have the " - "necessary packages for parallelization" - ) - syst_cores = multiprocessing.cpu_count() - cpu_percent = psutil.cpu_percent() - avail_cores = numpy.floor( - (100 - cpu_percent) / (100.0 / syst_cores) + def _rebuild_graph(self): + ParametricModelBase._rebuild_graph(self) + self._prepare_graph() + + def set_profile(self, profile, *args, **kwargs): + # generator doesn't accept xname, yname, dyname arguments + self._generator.set_profile(profile) + + @property + def operator(self): + logging.error("Accessing the operator of the generator.") + raise NotImplementedError() + + def evaluate(self): + if ( + not hasattr(self._generator, "profile") + or self._generator.profile is None + ): + raise RuntimeError( + "Unable to evaluate because the profile is not set. " + "Please set the profile before evaluating." ) - ncpu = int(numpy.max([1, avail_cores])) - pool = Pool(processes=ncpu) - self.calc_obj.parallel(ncpu=ncpu, mapfunc=pool.map) + return self._generator.operation() + + def residual(self): + ycalc = self.calc_obj.operation() + yobs = self.calc_obj.profile.ypar.value + dyobs = self.calc_obj.profile.dypar.value + return (ycalc - yobs) / dyobs - def _hide_dependent_parameters(self, use_uiso=True): - if use_uiso: + def _prepare_graph(self): + if self.use_uiso: dependent_par_names = [ r"\.U21$", r"\.U31$", @@ -319,30 +424,36 @@ def _hide_dependent_parameters(self, use_uiso=True): if regex.search(par_name): self._graph.nodes[par_name]["constrained_or_constant"] = True - def constrain_symmetry(self, spacegroup_symbol=None, use_uiso=True): - if spacegroup_symbol is None: - spacegroup_symbol = self.space_group_symbol - if isinstance(self.calc_obj.phase, DiffpyStructureParSet): - space_group_parset = constrain_as_space_group( - self.calc_obj.phase, spacegroup_symbol - ) - self._hide_dependent_parameters(use_uiso=use_uiso) - elif isinstance(self.calc_obj.phase, ObjCrystCrystalParSet): - if use_uiso is not False: - use_uiso = False + def _set_parallel(self, run_parallel): + if run_parallel: + try: + import multiprocessing + from multiprocessing import Pool + + import psutil + except ImportError: logger.warning( - "ObjCrystCrystalParSet prefers using the letter B instead " - "U for ADP parameters." + "Parallelization is disabled because the required " + "packages are not installed." ) - self._hide_dependent_parameters(use_uiso=use_uiso) - else: - raise ValueError( - "Unsupported calculation object type." - "Currently supported types are " - "DiffpyStructureParSet and ObjCrystCrystalParSet." + return + syst_cores = multiprocessing.cpu_count() + cpu_percent = psutil.cpu_percent() + avail_cores = numpy.floor( + (100 - cpu_percent) / (100.0 / syst_cores) ) + ncpu = int(numpy.max([1, avail_cores])) + pool = Pool(processes=ncpu) + self._generator.parallel(ncpu=ncpu, mapfunc=pool.map) + + def constrain_symmetry(self, spacegroup_symbol=None): + if spacegroup_symbol is None: + spacegroup_symbol = self.space_group_symbol + space_group_parset = constrain_as_space_group( + self._generator.phase, spacegroup_symbol + ) # hide constrained parameters in the graph - if use_uiso: + if self.use_uiso: symmetry_par_names = [ r"\.a$", r"\.b$", @@ -381,15 +492,15 @@ def constrain_symmetry(self, spacegroup_symbol=None, use_uiso=True): r"\.B23$", ] free_variables = [] - if isinstance(self.calc_obj.phase, DiffpyStructureParSet): + if isinstance(self._generator.phase, DiffpyStructureParSet): for latpar in space_group_parset.latpars: free_variables.append(latpar) for adpar in space_group_parset.adppars: free_variables.append(adpar) for xyzpar in space_group_parset.xyzpars: free_variables.append(xyzpar) - elif isinstance(self.calc_obj.phase, ObjCrystCrystalParSet): - for par in self.calc_obj.phase.sgpars: + elif isinstance(self._generator.phase, ObjCrystCrystalParSet): + for par in self._generator.phase.sgpars: free_variables.append(par) for i in range(len(free_variables)): while isinstance(free_variables[i], ParameterProxy): @@ -397,7 +508,7 @@ def constrain_symmetry(self, spacegroup_symbol=None, use_uiso=True): organized_name = ".".join( [ obj.name - for obj in self.calc_obj._locate_managed_object( + for obj in self._generator._locate_managed_object( free_variables[i] ) ] @@ -412,13 +523,16 @@ def constrain_symmetry(self, spacegroup_symbol=None, use_uiso=True): self._graph.nodes[par_name][ "constrained_or_constant" ] = True + else: + self.free_sgpar_names.append(par_name) + self.sgpar_names.append(par_name) def check_molecule_or_molatom(func): @wraps(func) # preserves name, docstring, signature metadata def wrapper(self, *args, **kwargs): if not ( - isinstance(self.calc_obj.phase, ObjCrystMoleculeParSet) - or isinstance(self.calc_obj.phase, ObjCrystMolAtomParSet) + isinstance(self._generator.phase, ObjCrystMoleculeParSet) + or isinstance(self._generator.phase, ObjCrystMolAtomParSet) ): logging.warning( "The method %s is only applicable to " @@ -442,7 +556,7 @@ def add_bond_length_parameter( const=None, parent_node_name=None, ): - phase = self.calc_obj.phase + phase = self._generator.phase par = phase.addBondLengthParameter( par_name, atom1, atom2, value, const ) @@ -463,7 +577,7 @@ def add_bond_angle_parameter( const=None, parent_node_name=None, ): - phase = self.calc_obj.phase + phase = self._generator.phase par = phase.addBondAngleParameter( par_name, atom1, atom2, atom3, value, const ) @@ -485,7 +599,7 @@ def add_dihedral_angle_parameter( const=None, parent_node_name=None, ): - phase = self.calc_obj.phase + phase = self._generator.phase par = phase.addDihedralAngleParameter( par_name, atom1, atom2, atom3, atom4, value, const ) @@ -499,46 +613,23 @@ def add_dihedral_angle_parameter( def restrain_bond_length_parameter( self, par, length, sigma, delta, scaled=False ): - phase = self.calc_obj.phase + phase = self._generator.phase phase.restrainBondLengthParameter(par, length, sigma, delta, scaled) @check_molecule_or_molatom def restrain_bond_angle_parameter( self, par, angle, sigma, delta, scaled=False ): - phase = self.calc_obj.phase + phase = self._generator.phase phase.restrainBondAngleParameter(par, angle, sigma, delta, scaled) @check_molecule_or_molatom def restrain_dihedral_angle_parameter( self, par, angle, sigma, delta, scaled=False ): - phase = self.calc_obj.phase + phase = self._generator.phase phase.restrainDihedralAngleParameter(par, angle, sigma, delta, scaled) - def _rebuild_graph(self): - old_graph = self._graph - self._graph = nx.DiGraph() - self._construct_parameter_graph( - # PDFGenerator itself holds parameters - self.calc_obj, - prefix="", - old_graph=old_graph, - ) - - def set_profile(self, profile): - self.calc_obj.set_profile(profile) - # no submodel is allowed ParametricModelPDF - - def evaluate(self): - return self.calc_obj.operation() - - def residual(self): - ycalc = self.calc_obj.operation() - yobs = self.calc_obj.profile.ypar.value - dyobs = self.calc_obj.profile.dypar.value - return (ycalc - yobs) / dyobs - # NOTE: certain space groups require dual origin handling. DUAL_ORIGIN_SG_NUMBERS = { @@ -572,8 +663,8 @@ def residual(self): def create_pdf_model_from_file( name, structure_file_path, + finite, library="Diffpy", - finite=False, run_parallel=True, ): """Create a ParametricModelPDF by parsing a structure file.""" @@ -594,12 +685,13 @@ def create_pdf_model_from_file( ) -def create_pdf_model_from_model(name, from_model, run_parallel=True): +def create_pdf_model_from_model(name, from_model, finite, run_parallel=True): """Create a ParametricModelPDF sharing the phase of from_model.""" return ParametricModelPDF( name, from_model.calc_obj.phase, spacegroup_symbol=from_model.space_group_symbol, + finite=finite, run_parallel=run_parallel, ) @@ -607,10 +699,10 @@ def create_pdf_model_from_model(name, from_model, run_parallel=True): def create_pdf_model_from_code( name, code, + finite, spacegroup_symbol="P1", - global_namespace={}, + global_namespace=None, local_structure_name="structure", - finite=False, run_parallel=True, ): """Create a ParametricModelPDF by executing code that builds a @@ -619,6 +711,8 @@ def create_pdf_model_from_code( The code must assign the structure/crystal object to a variable named structure_name (default "structure") in its local namespace. """ + if global_namespace is None: + global_namespace = {} local_namespace = {} exec(code, global_namespace, local_namespace) if local_structure_name not in local_namespace: diff --git a/src/diffpy/apps/refinebase/refinement_server.py b/src/diffpy/apps/parametric_refinement/refinement_server.py similarity index 87% rename from src/diffpy/apps/refinebase/refinement_server.py rename to src/diffpy/apps/parametric_refinement/refinement_server.py index 34560d9..4b6b580 100644 --- a/src/diffpy/apps/refinebase/refinement_server.py +++ b/src/diffpy/apps/parametric_refinement/refinement_server.py @@ -9,7 +9,9 @@ from mcp.server import MCPServer from mcp.server.mcpserver.exceptions import ToolError -from diffpy.apps.refinebase.refinement_session import RefinementSession +from diffpy.apps.parametric_refinement.refinement_session import ( + RefinementSession, +) session = RefinementSession() mcp = MCPServer("diffpy.apps") @@ -146,8 +148,8 @@ async def remove_profile( @mcp.tool() @tool_errors async def add_equation_model( + equation_string: Annotated[str, "Equation for the parametric model"], model_name: Annotated[str, "Name of the parametric model"] = uuid.uuid4(), - equation_str: Annotated[str, "Equation for the parametric model"] = None, ) -> str: """Add an equation-based parametric model to the refinement session. @@ -155,11 +157,11 @@ async def add_equation_model( ---------- model_name : str Name of the parametric model to add. - equation_str : str, optional + equation_string : str Equation for the parametric model. e.g. "a * x + b" """ session.add_equation_model( - equation_str=equation_str, + equation_string=equation_string, model_name=model_name, ) return f"Model {model_name} added successfully." @@ -168,6 +170,9 @@ async def add_equation_model( @mcp.tool() @tool_errors async def add_pdf_model( + finite: Annotated[ + bool, "Whether to use DebyePDFGenerator instead of PDFGenerator" + ], model_name: Annotated[str, "Name of the parametric model"] = uuid.uuid4(), structure_file_path: Annotated[str, "Path to the structure file"] = None, from_model_name: Annotated[ @@ -181,13 +186,10 @@ async def add_pdf_model( ] = "Diffpy", global_namespace: Annotated[ dict, "Global namespace to execute 'code' in" - ] = {}, + ] = None, local_structure_name: Annotated[ str, "Name of the structure variable assigned by 'code'" ] = "structure", - finite: Annotated[ - bool, "Whether to use DebyePDFGenerator instead of PDFGenerator" - ] = False, run_parallel: Annotated[ bool, "Whether to run the PDF model in parallel" ] = True, @@ -292,7 +294,18 @@ async def set_model_equation( @tool_errors async def get_model_evaluation( model_name: Annotated[str, "Name of the parametric model"], - data_path: Annotated[str, "Path to the data to retrieve"], + data_path: Annotated[ + str, "The path to the file where the evaluation will be written" + ], + profile_name: Annotated[ + str, + ( + "Name of the profile to use for the evaluation. " + "If None, temporary profile will be created for the evaluation. " + "If both profile_name and xmin/xmax/dx are provided, " + "the profile's calculation range will be updated accordingly." + ), + ] = None, xmin: Annotated[ float | None, ( @@ -314,6 +327,13 @@ async def get_model_evaluation( "if the profile is not set for the model" ), ] = None, + meta: Annotated[ + dict | None, + ( + "Metadata to associate with the evaluation. " + "If None, no additional metadata will be added." + ), + ] = None, xname: Annotated[ str, ( @@ -323,9 +343,16 @@ async def get_model_evaluation( ] = "r", ) -> str: """Get the current evaluation of a parametric model.""" - evaluation = session.get_model_evaluation( - model_name=model_name, xmin=xmin, xmax=xmax, dx=dx, xname=xname - ).tolist() + r, ycalc = session.get_model_evaluation( + model_name=model_name, + profile_name=profile_name, + xmin=xmin, + xmax=xmax, + dx=dx, + xname=xname, + meta=meta, + ) + evaluation = {"r": r.tolist(), "ycalc": ycalc.tolist()} with open(data_path, "w") as f: json.dump(evaluation, f) return ( @@ -543,7 +570,19 @@ async def solve( include_sgpars: Annotated[ bool, "Whether to also include sgpars from the models automatically" ] = False, -) -> str: + method: Annotated[str, "Optimizer method: trf, dogbox, or lm"] = "trf", + x_scale: Annotated[ + float | str, "Optimizer parameter scaling, or 'jac'" + ] = 1.0, + max_nfev: Annotated[ + int | None, "Maximum number of function evaluations" + ] = None, + ftol: Annotated[float, "Cost-function termination tolerance"] = 1e-8, + xtol: Annotated[float, "Step-size termination tolerance"] = 1e-8, + background: Annotated[ + bool, "Run asynchronously and return a job ID" + ] = False, +) -> dict | str: """Initiate a refinement using the specified profiles, models, and variables. @@ -586,6 +625,17 @@ async def solve( List of metadata dictionaries for each profile. include_sgpars : bool, optional Whether to also include sgpars from the models automatically. + method : str, optional + Least-squares optimizer method. + x_scale : float or str, optional + Parameter scaling used by the optimizer. + max_nfev : int, optional + Maximum number of residual evaluations. + ftol, xtol : float, optional + Optimizer termination tolerances. + background : bool, optional + If True, return a job ID immediately and run the solve in the + background. Notes ----- @@ -609,10 +659,32 @@ async def solve( weights=weights, metas=metas, include_sgpars=include_sgpars, + method=method, + x_scale=x_scale, + max_nfev=max_nfev, + ftol=ftol, + xtol=xtol, + background=background, ) return out_string +@mcp.tool() +@tool_errors +async def get_solve_status( + job_id: Annotated[str, "Background refinement job ID"], +) -> dict: + """Return the status and result of a background refinement job.""" + return session.get_job_status(job_id) + + +@mcp.tool() +@tool_errors +async def list_running_jobs() -> list[dict]: + """List pending and currently running background refinements.""" + return session.list_running_jobs() + + if __name__ == "__main__": mcp.run() diff --git a/src/diffpy/apps/refinebase/refinement_session.py b/src/diffpy/apps/parametric_refinement/refinement_session.py similarity index 77% rename from src/diffpy/apps/refinebase/refinement_session.py rename to src/diffpy/apps/parametric_refinement/refinement_session.py index 1253655..d5eec77 100644 --- a/src/diffpy/apps/refinebase/refinement_session.py +++ b/src/diffpy/apps/parametric_refinement/refinement_session.py @@ -1,11 +1,13 @@ +import logging import uuid from collections import OrderedDict from collections.abc import Callable +from concurrent.futures import ThreadPoolExecutor import numpy -from scipy.optimize import leastsq +from scipy.optimize import least_squares -from diffpy.apps.refinebase.parametric_model import ( +from diffpy.apps.parametric_refinement.parametric_model import ( ParametricModelEquation, ParametricModelPDF, ) @@ -16,12 +18,16 @@ Profile, ) +logger = logging.getLogger(__name__) + class RefinementSession: def __init__(self): self.recipes_dict = OrderedDict() self.profiles_dict = OrderedDict() self.models_dict = OrderedDict() + self.jobs_dict = {} + self._job_executor = ThreadPoolExecutor() def add_profile_from_file( self, @@ -102,34 +108,29 @@ def set_profile_calculation_points(self, profile_name: str, x): profile = self._get_profile(profile_name) profile.set_calculation_points(x) - def add_equation_model(self, model_name: str, equation_str=None): - from diffpy.apps.refinebase.parametric_model import ( + def add_equation_model(self, model_name: str, equation_string: str): + from diffpy.apps.parametric_refinement.parametric_model import ( ParametricModelEquation, ) if model_name in self.models_dict: raise ValueError(f"Model with ID {model_name} already exists.") - if equation_str is not None: - model = ParametricModelEquation(model_name, equation_str) - else: - raise ValueError( - "Either equation_str or from_model must be provided." - ) + model = ParametricModelEquation(model_name, equation_string) self.models_dict[model_name] = model def add_pdf_model( self, model_name: str, + finite: bool, structure_file_path=None, from_model_name=None, code=None, library="Diffpy", - global_namespace={}, + global_namespace=None, local_structure_name="structure", - finite=False, run_parallel=True, ): - from diffpy.apps.refinebase.parametric_model import ( + from diffpy.apps.parametric_refinement.parametric_model import ( create_pdf_model_from_code, create_pdf_model_from_file, create_pdf_model_from_model, @@ -154,6 +155,7 @@ def add_pdf_model( pdf_model = create_pdf_model_from_model( model_name, from_model, + finite=finite, run_parallel=run_parallel, ) elif code is not None: @@ -178,7 +180,7 @@ def add_function_model( function: Callable | str, argnames: list[str] | None = None, ): - from diffpy.apps.refinebase.parametric_model import ( + from diffpy.apps.parametric_refinement.parametric_model import ( ParametricModelFunction, ) @@ -248,35 +250,59 @@ def get_model_residual(self, model_name: str): return model.residual() def get_model_evaluation( - self, model_name: str, xmin=None, xmax=None, dx=None, xname="r" + self, + model_name: str, + profile_name: str | None = None, + xmin=None, + xmax=None, + dx=None, + xname="r", + meta: dict | None = None, ): model = self._get_model(model_name) if not hasattr(model, "evaluate"): raise ValueError( f"Model '{model_name}' does not have an evaluate method." ) - if not ( - hasattr(model, "calc_obj") - and hasattr(model.calc_obj, "profile") - and model.calc_obj.profile is not None - ): - profile = Profile() - if isinstance(model.calc_obj, FitContribution): - profile.set_observed_profile( - xobs=numpy.arange(xmin, xmax, dx), - yobs=numpy.zeros_like(numpy.arange(xmin, xmax, dx)), - xname=xname, + if profile_name is not None: + try: + profile = self._get_profile(profile_name) + except ValueError: + logger.warning( + f"Profile '{profile_name}' not found in the session. " + "A temporary profile will be created according to " + "provided xmin, xmax, and dx." ) + profile = None + if profile_name is None: + if ( + hasattr(model, "calc_obj") + and hasattr(model.calc_obj, "profile") + and model.calc_obj.profile is not None + ): + profile = model.calc_obj.profile else: - # PDFGenerator uses profile.x, so it doesn't accept xname + if all(v is None for v in (xmin, xmax, dx)): + logger.error( + "No calculation range specified (xmin, xmax, dx are " + "all None). Program cannot determine the independent " + "variable 'x'. Please specify xmin, xmax, and dx if " + "profile is not existing or provided." + ) + profile = Profile() profile.set_observed_profile( xobs=numpy.arange(xmin, xmax, dx), yobs=numpy.zeros_like(numpy.arange(xmin, xmax, dx)), ) + profile.set_calculation_range(xmin=xmin, xmax=xmax, dx=dx) + profile.meta.update(meta or {}) + if isinstance(model, ParametricModelEquation): + model.set_profile(profile, xname=xname) + else: model.set_profile(profile) + r = profile.x ycalc = model.evaluate() - model.calc_obj.profile = None - return ycalc + return r, ycalc def constrain_pdf_model_space_group_symmetry( self, model_name, space_group=None @@ -473,6 +499,11 @@ def _solve( weights=None, residual_equations=None, metas=None, + method="trf", + x_scale=1.0, + max_nfev=None, + ftol=1e-8, + xtol=1e-8, ): recipe = FitRecipe() self.recipes_dict[name] = recipe @@ -512,10 +543,12 @@ def _solve( variables = [self.get_variable(name)["obj"] for name in variable_names] variable_names = ["_".join(name.split(".")) for name in variable_names] + tag_manager = recipe._tagmanager for i, var in enumerate(variables): if var in recipe._parameters.values(): + tag_manager.tag(var, "listed_var") continue - recipe.add_variable(var, name=variable_names[i]) + recipe.add_variable(var, name=variable_names[i], tag="listed_var") for eq_or_var_name, arg_dict in bounds.items(): lb = arg_dict.get("lower_bound", -numpy.inf) ub = arg_dict.get("upper_bound", numpy.inf) @@ -530,14 +563,57 @@ def _solve( else: par = self.get_variable(eq_or_var_name)["obj"] par.bound_range(lb, ub) - recipe.free("all") - leastsq(recipe.residual, recipe.get_values()) + if len(recipe.get_values()) != 0: + recipe.fix("all") + recipe.free("listed_var") + result = least_squares( + recipe.residual, + recipe.get_values(), + method=method, + x_scale=x_scale, + max_nfev=max_nfev, + ftol=ftol, + xtol=xtol, + ) # NOTE: non-scalar value will raise error in `get_results_string` try: result_string = FitResults(recipe).get_results_string() except TypeError: result_string = "Refinement Finished." - return result_string + + if constraints: + for constraint_name, constraint_eq_str in constraints[1].items(): + constraint_var = self.get_variable(constraint_name)["obj"] + recipe.unconstrain(constraint_var) + return { + "success": bool(result.success), + "status": int(result.status), + "message": result.message, + "nfev": int(result.nfev), + "cost": float(result.cost), + "result": result_string, + "meta": { + models[i].name: profiles[i].meta for i in range(len(models)) + }, + } + + def get_job_status(self, job_id): + """Return the current status and result of a background + solve.""" + try: + job = self.jobs_dict[job_id] + except KeyError as exc: + raise ValueError(f"Job '{job_id}' not found.") from exc + return dict(job) + + def list_running_jobs(self): + """Return background jobs that are pending or currently + running.""" + return [ + dict(job) + for job in self.jobs_dict.values() + if job["status"] in {"pending", "running"} + ] def solve( self, @@ -547,11 +623,21 @@ def solve( residual_equations=None, constraints=None, bounds=None, - name=uuid.uuid4(), + name=None, weights=None, metas=None, include_sgpars=False, + method="trf", + x_scale=1.0, + max_nfev=None, + ftol=1e-8, + xtol=1e-8, + background=False, ): + if name is None: + name = str(uuid.uuid4()) + if variable_names is None: + variable_names = [] profiles = [] for profile_name in profile_names: if profile_name not in self.profiles_dict: @@ -571,29 +657,56 @@ def solve( if include_sgpars: for model in models: if isinstance(model, ParametricModelPDF): - for sgpar_name in model.sgpar_names: + for sgpar_name in model.free_sgpar_names: if sgpar_name in variable_names: continue variable_names.append(sgpar_name) else: for submodel in model._submodels: if isinstance(submodel, ParametricModelPDF): - for sgpar_name in submodel.sgpar_names: + for sgpar_name in submodel.free_sgpar_names: if sgpar_name in variable_names: continue variable_names.append(sgpar_name) - return self._solve( - profiles=profiles, - models=models, - variable_names=variable_names, - residual_equations=residual_equations, - constraints=constraints, - bounds=bounds, - name=name, - weights=weights, - metas=metas, - ) + solve_args = { + "profiles": profiles, + "models": models, + "variable_names": variable_names, + "residual_equations": residual_equations, + "constraints": constraints, + "bounds": bounds, + "name": name, + "weights": weights, + "metas": metas, + "method": method, + "x_scale": x_scale, + "max_nfev": max_nfev, + "ftol": ftol, + "xtol": xtol, + } + if not background: + return self._solve(**solve_args) + + job_id = str(uuid.uuid4()) + self.jobs_dict[job_id] = { + "job_id": job_id, + "status": "pending", + "result": None, + "error": None, + } + + def run_job(): + self.jobs_dict[job_id]["status"] = "running" + try: + self.jobs_dict[job_id]["result"] = self._solve(**solve_args) + self.jobs_dict[job_id]["status"] = "completed" + except Exception as exc: + self.jobs_dict[job_id]["error"] = str(exc) + self.jobs_dict[job_id]["status"] = "failed" + + self._job_executor.submit(run_job) + return job_id def plot(self): # NOTE: to be implemented diff --git a/tests/conftest.py b/tests/conftest.py index cd237de..89a4ba9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -26,7 +26,9 @@ def user_filesystem(tmp_path): @pytest.fixture def nested_sine_model(): - from diffpy.apps.refinebase.parametric_model import ParametricModelEquation + from diffpy.apps.parametric_refinement.parametric_model import ( + ParametricModelEquation, + ) submodel = ParametricModelEquation("sub", "a*x") model = ParametricModelEquation("main", "A*sin(u)") @@ -41,3 +43,26 @@ def sine_profile(): sine_profile = Profile() sine_profile.set_observed_profile(xobs, yobs) return sine_profile + + +@pytest.fixture +def mock_clone_skills(user_filesystem, mocker): + def fake_clone(cmd, **kwargs): + # the last argument is the temporary directory to clone into + target_dir = Path(cmd[-1]) + (target_dir / "skills.json").write_text( + json.dumps(["diffpy-playbook"]) + ) + skill_dir = target_dir / "diffpy-playbook" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text("skill") + + mocker.patch("diffpy.apps.app_agentify.ensure_setup", return_value=True) + mocker.patch( + "diffpy.apps.app_agentify.subprocess.run", side_effect=fake_clone + ) + mocker.patch.object( + Path, "home", return_value=user_filesystem / "home_dir" + ) + mocker.patch.object(Path, "cwd", return_value=user_filesystem / "cwd_dir") + return user_filesystem diff --git a/tests/test_agentify.py b/tests/test_agentify.py index 45aa7b3..7a0dde4 100644 --- a/tests/test_agentify.py +++ b/tests/test_agentify.py @@ -1,18 +1,10 @@ import re -import tempfile from pathlib import Path from types import SimpleNamespace -from unittest import mock import pytest -from diffpy.apps.app_agentify import agentify, ensure_setup - -pytestmark = pytest.mark.skipif( - not ensure_setup(), - reason="Internet connection or git unavailable. Skipping agentify tests.", - allow_module_level=True, -) +from diffpy.apps.app_agentify import agentify @pytest.mark.parametrize( @@ -68,52 +60,44 @@ ), ], ) -def test_agentify(args, expected_scope, expected_skill_dir): - with tempfile.TemporaryDirectory() as tmp: - with ( - mock.patch.object(Path, "home", return_value=Path(tmp) / "home"), - mock.patch.object(Path, "cwd", return_value=Path(tmp) / "cwd"), - ): - agentify(args) - expected_path = Path(tmp) / expected_scope / expected_skill_dir - assert expected_path.exists() +def test_agentify(args, expected_scope, expected_skill_dir, mock_clone_skills): + agentify(args) + scope_dir = {"cwd": "cwd_dir", "home": "home_dir"}[expected_scope] + expected_path = mock_clone_skills / scope_dir / expected_skill_dir + assert expected_path.exists() -def test_agentify_update(): - with tempfile.TemporaryDirectory() as tmp: - with ( - mock.patch.object(Path, "home", return_value=Path(tmp) / "home"), - mock.patch.object(Path, "cwd", return_value=Path(tmp) / "cwd"), - ): - # C1: Deploy again without --update flag when skill already exists. - # Expect FileExistsError to be raised, and the error message - # matches. - args = SimpleNamespace( - agent="claude", - system=False, - update=False, - ) - agentify(args) - skill_path = ( - Path(tmp) / "cwd" / ".claude" / "skills" / "diffpy-playbook" - ) - assert skill_path.exists() - args.update = True - agentify(args) - pytest.raises( - FileExistsError, - match=re.escape( - "Agentic skill diffpy-playbook already exists at " - f"{skill_path}. " - "To overwrite, pass '--update' flag to update the skill" - ), - ) - # C2: Deploy again with --update flag when skill already exists - # with a dummy file in the skill directory. - # Expect no error to be raised, and the skill is updated. - dummy_file = skill_path / "dummy.txt" - dummy_file.touch() - assert dummy_file.exists() - args.update = True - agentify(args) - assert not dummy_file.exists() +def test_agentify_update(mock_clone_skills): + tmp = mock_clone_skills + # C1: Deploy again without --update flag when skill already exists. + # Expect FileExistsError to be raised, and the error message + # matches. + args = SimpleNamespace( + agent="claude", + system=False, + update=False, + ) + agentify(args) + skill_path = ( + Path(tmp) / "cwd_dir" / ".claude" / "skills" / "diffpy-playbook" + ) + assert skill_path.exists() + args.update = True + agentify(args) + pytest.raises( + FileExistsError, + match=re.escape( + "Agentic skill diffpy-playbook already exists at " + f"{skill_path}. " + "To overwrite, pass '--update' flag to update the skill" + ), + ) + # C2: Deploy again with --update flag when skill already exists + # with a dummy file in the skill directory. + # Expect no error to be raised, and the skill is updated. + dummy_file = skill_path / "dummy.txt" + dummy_file.touch() + assert dummy_file.exists() + args.update = True + agentify(args) + assert not dummy_file.exists() diff --git a/tests/test_parametric_model.py b/tests/test_parametric_model.py index f0e074a..0cc87ab 100644 --- a/tests/test_parametric_model.py +++ b/tests/test_parametric_model.py @@ -3,7 +3,7 @@ import numpy import pytest -from diffpy.apps.refinebase.parametric_model import ( +from diffpy.apps.parametric_refinement.parametric_model import ( ParametricModelEquation, ParametricModelFunction, create_pdf_model_from_file, @@ -71,7 +71,9 @@ def test_parametric_pdf_model_parameters(): profile.load_parsed_data(parser) profile.set_calculation_range(xmax=20) pdf_model = create_pdf_model_from_file( - "ni", structure_file_path=str(_DATA_DIR / "Ni.cif") + "ni", + structure_file_path=str(_DATA_DIR / "Ni.cif"), + finite=False, ) parameter_names = [ "ni.phase.lattice.a", diff --git a/tests/test_refinement_server.py b/tests/test_refinement_server.py index 0805d8d..966a537 100644 --- a/tests/test_refinement_server.py +++ b/tests/test_refinement_server.py @@ -9,7 +9,7 @@ ) from mcp import Client -from diffpy.apps.refinebase.refinement_server import mcp +from diffpy.apps.parametric_refinement.refinement_server import mcp _DATA_DIR = Path(__file__).parent / "data" @@ -23,7 +23,7 @@ def anyio_backend(): def clear_session(): # The MCP server uses a module-level singleton session; reset it so # models/profiles from one test don't leak into the next. - from diffpy.apps.refinebase.refinement_server import session + from diffpy.apps.parametric_refinement.refinement_server import session session.clear() yield @@ -34,7 +34,7 @@ def clear_session(): async def test_refine_sine(): # C1: Set up the MCP client and do a sine refinement # Expect all objs are created and refined successfully - from diffpy.apps.refinebase.refinement_server import session + from diffpy.apps.parametric_refinement.refinement_server import session async with Client(mcp, raise_exceptions=True) as mcp_client: await mcp_client.call_tool( @@ -49,7 +49,7 @@ async def test_refine_sine(): "add_equation_model", { "model_name": "sine_model", - "equation_str": "A*sin(x)", + "equation_string": "A*sin(x)", }, ) assert "sine_model" in session.models_dict @@ -77,7 +77,7 @@ async def test_refine_sine(): @pytest.mark.anyio async def test_refine_ni(): - from diffpy.apps.refinebase.refinement_server import session + from diffpy.apps.parametric_refinement.refinement_server import session ni_refined_parameters = run_ni_example() @@ -110,6 +110,7 @@ async def test_refine_ni(): { "model_name": "pdf", "structure_file_path": str(_DATA_DIR / "Ni.cif"), + "finite": False, }, ) await mcp_client.call_tool( @@ -123,7 +124,7 @@ async def test_refine_ni(): "add_equation_model", { "model_name": "ni_model", - "equation_str": "s*pdf", + "equation_string": "s*pdf", }, ) await mcp_client.call_tool( @@ -176,7 +177,7 @@ async def test_refine_ni(): @pytest.mark.anyio async def test_refine_multi_contribution(): - from diffpy.apps.refinebase.refinement_server import session + from diffpy.apps.parametric_refinement.refinement_server import session multi_contribution_refined_parameters = run_multi_contribution_example() @@ -224,6 +225,7 @@ async def test_refine_multi_contribution(): { "structure_file_path": str(_DATA_DIR / "Ni.cif"), "model_name": "pdf_ni", + "finite": False, }, ) await mcp_client.call_tool( @@ -232,17 +234,26 @@ async def test_refine_multi_contribution(): ) await mcp_client.call_tool( "add_pdf_model", - {"from_model_name": "pdf_ni", "model_name": "pdf_ni_neutron"}, + { + "from_model_name": "pdf_ni", + "model_name": "pdf_ni_neutron", + "finite": False, + }, ) await mcp_client.call_tool( "add_pdf_model", - {"from_model_name": "pdf_ni", "model_name": "pdf_ni_partial"}, + { + "from_model_name": "pdf_ni", + "model_name": "pdf_ni_partial", + "finite": False, + }, ) await mcp_client.call_tool( "add_pdf_model", { "structure_file_path": str(_DATA_DIR / "si.cif"), "model_name": "pdf_si", + "finite": False, }, ) await mcp_client.call_tool( @@ -251,13 +262,17 @@ async def test_refine_multi_contribution(): ) await mcp_client.call_tool( "add_pdf_model", - {"from_model_name": "pdf_si", "model_name": "pdf_si_partial"}, + { + "from_model_name": "pdf_si", + "model_name": "pdf_si_partial", + "finite": False, + }, ) await mcp_client.call_tool( "add_equation_model", { "model_name": "main", - "equation_str": "scale * (pdf_ni_partial + pdf_si_partial)", + "equation_string": "scale * (pdf_ni_partial + pdf_si_partial)", }, ) await mcp_client.call_tool( @@ -357,7 +372,7 @@ async def test_refine_multi_contribution(): @pytest.mark.anyio async def test_refine_nanoparticle_example(): - from diffpy.apps.refinebase.refinement_server import session + from diffpy.apps.parametric_refinement.refinement_server import session refined_parameters = run_nanoparticle_example() @@ -390,6 +405,7 @@ async def test_refine_nanoparticle_example(): { "model_name": "pdf", "structure_file_path": str(_DATA_DIR / "pb.cif"), + "finite": False, }, ) await mcp_client.call_tool( @@ -398,7 +414,7 @@ async def test_refine_nanoparticle_example(): ) await mcp_client.call_tool( "add_equation_model", - {"model_name": "main", "equation_str": "f*pdf"}, + {"model_name": "main", "equation_string": "f*pdf"}, ) await mcp_client.call_tool( "combine_models", diff --git a/tests/test_refinement_session.py b/tests/test_refinement_session.py index e72f71e..d7fa50b 100644 --- a/tests/test_refinement_session.py +++ b/tests/test_refinement_session.py @@ -8,7 +8,9 @@ run_ni_example, ) -from diffpy.apps.refinebase.refinement_session import RefinementSession +from diffpy.apps.parametric_refinement.refinement_session import ( + RefinementSession, +) _DATA_DIR = Path(__file__).parent / "data" @@ -22,7 +24,9 @@ def test_bonds(): session.add_profile_from_arrays( profile_name="linear", xarray=xarray, yarray=yarray ) - session.add_equation_model(model_name="linear_model", equation_str="m*x") + session.add_equation_model( + model_name="linear_model", equation_string="m*x" + ) session.set_variables_value( { "linear_model.m": 1, @@ -61,8 +65,8 @@ def test_bonds(): def test_refine_sine(sine_profile): # C1: Refinement session without additional calculator or functions session = RefinementSession() - session.add_equation_model(model_name="sub", equation_str="a*x") - session.add_equation_model(model_name="main", equation_str="A*sin(sub)") + session.add_equation_model(model_name="sub", equation_string="a*x") + session.add_equation_model(model_name="main", equation_string="A*sin(sub)") session.combine_models(parent_model_name="main", child_model_names=["sub"]) session.set_variables_value( name_value_dict={ @@ -109,6 +113,7 @@ def test_refine_ni(): session.add_pdf_model( model_name="pdf", structure_file_path=str(_DATA_DIR / "Ni.cif"), + finite=False, ) session.constrain_pdf_model_space_group_symmetry( model_name="pdf", space_group="Fm-3m" @@ -155,7 +160,7 @@ def test_refine_ni(): session.set_variables_value({"pdf.scale": 1}) session.add_equation_model( model_name="ni_model", - equation_str="s*pdf", + equation_string="s*pdf", ) session.combine_models( parent_model_name="ni_model", child_model_names=["pdf"] @@ -223,28 +228,33 @@ def test_refine_multi_contribution(): session.add_pdf_model( structure_file_path=str(_DATA_DIR / "Ni.cif"), model_name="pdf_ni", + finite=False, ) session.constrain_pdf_model_space_group_symmetry("pdf_ni") session.add_pdf_model( from_model_name="pdf_ni", model_name="pdf_ni_neutron", + finite=False, ) session.add_pdf_model( from_model_name="pdf_ni", model_name="pdf_ni_partial", + finite=False, ) session.add_pdf_model( structure_file_path=str(_DATA_DIR / "si.cif"), model_name="pdf_si", + finite=False, ) session.constrain_pdf_model_space_group_symmetry("pdf_si") session.add_pdf_model( from_model_name="pdf_si", model_name="pdf_si_partial", + finite=False, ) session.add_equation_model( model_name="main", - equation_str="scale * (pdf_ni_partial + pdf_si_partial)", + equation_string="scale * (pdf_ni_partial + pdf_si_partial)", ) session.combine_models( parent_model_name="main", @@ -352,9 +362,10 @@ def test_refine_nanoparticle_example(): session.add_pdf_model( model_name="pdf", structure_file_path=str(_DATA_DIR / "pb.cif"), + finite=False, ) session.constrain_pdf_model_space_group_symmetry(model_name="pdf") - session.add_equation_model(model_name="main", equation_str="f*pdf") + session.add_equation_model(model_name="main", equation_string="f*pdf") session.combine_models( parent_model_name="main", child_model_names=["f", "pdf"] )