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

Improve print_parameter_info functionality #3361

Closed
eibar-flores opened this issue Sep 20, 2023 · 6 comments · Fixed by #3584
Closed

Improve print_parameter_info functionality #3361

eibar-flores opened this issue Sep 20, 2023 · 6 comments · Fixed by #3584
Assignees
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours feature priority: medium To be resolved if time allows

Comments

@eibar-flores
Copy link

Description

Pybamm has an api to access parameter sets. When selecting a model (e.g. SPM), some, not all of the parameters of the set are used. It would be nice to have a method to pull only the parameters of the chosen model.

Motivation

No response

Possible Implementation

To have independent apis for parameter sets and models. E.g.

  • pybamm.ParameterValues("Chen2020") to get the whole set (keys and values)
  • pybamm.pybamm.lithium_ion.DFN().parameters to get the parameters particular to the model (only the keys)
  • pybamm.Simulation(model=model, parameter_values=parameters).parameter_values to get the parameters particular to the model and the values it pulled from the set

Additional context

No response

@brosaplanella
Copy link
Member

What case do you have in mind for this particular functionality? At the moment, models have the print_parameter_info() method which prints the parameters needed for that model. This provides the list of required parameters as a string, so you know which parameters you need to provide in a new parameter set.

@ejfdickinson
Copy link

ejfdickinson commented Sep 28, 2023

@brosaplanella Where is the print_parameter_info() method documented? It doesn't seem to be mentioned in the pybamm.BaseModel documentation.

This is a very helpful sounding function that I never knew existed. So while it probably satisfies this issue, it's not surprising that it might be missed.

@eibar-flores
Copy link
Author

Thanks for your quick reply @brosaplanella. As I understand, parameter set, model and simulation each has a subset of battery parameters, and these battery parameters interact somehow in the background when calling simulations. For instance: I assume that calling pybamm.Simulation(model=model, parameter_values=parameters) pulls a subset from pybamm.ParameterValues("Chen2020") and changes defaults in pybamm.pybamm.lithium_ion.DFN(). Hence, having separate methods to access these battery parameters is useful. Thanks for print_parameter_info(), it solves the issue for models (e.g. pybamm.pybamm.lithium_ion.DFN()). But as @ejfdickinson says, it would be nice to expose these useful methods in the documentation.

@brosaplanella
Copy link
Member

brosaplanella commented Oct 2, 2023

@brosaplanella Where is the print_parameter_info() method documented? It doesn't seem to be mentioned in the pybamm.BaseModel documentation.

This is a very helpful sounding function that I never knew existed. So while it probably satisfies this issue, it's not surprising that it might be missed.

I just realised that it is not documented in the docs (opened #3392 for this), but it is demonstrated in this example https://docs.pybamm.org/en/latest/source/examples/notebooks/parameterization/parameter-values.html#Printing-parameter-values

@brosaplanella
Copy link
Member

brosaplanella commented Oct 2, 2023

Thanks for your quick reply @brosaplanella. As I understand, parameter set, model and simulation each has a subset of battery parameters, and these battery parameters interact somehow in the background when calling simulations. For instance: I assume that calling pybamm.Simulation(model=model, parameter_values=parameters) pulls a subset from pybamm.ParameterValues("Chen2020") and changes defaults in pybamm.pybamm.lithium_ion.DFN(). Hence, having separate methods to access these battery parameters is useful. Thanks for print_parameter_info(), it solves the issue for models (e.g. pybamm.pybamm.lithium_ion.DFN()). But as @ejfdickinson says, it would be nice to expose these useful methods in the documentation.

I think one way to improve print_parameter_info, would be to create a separate method called parameter_info (or something like that) which pulls out a list of the parameters used, and which then it can be passed to the dictionary to extract a subdictionary. Then, print_parameter_info could just be used to print that information in a more readable way. This would tie nicely with #1500 as well.

It is a fairly simple issue which is good for new contributors, if you want to give it a go. Let us know if you need any help!

@brosaplanella brosaplanella changed the title Display subset of parameters specific to a model Improve print_parameter_info functionality Oct 2, 2023
@brosaplanella brosaplanella added difficulty: easy A good issue for someone new. Can be done in a few hours priority: medium To be resolved if time allows hacktoberfest labels Oct 2, 2023
@cringeyburger
Copy link
Contributor

cringeyburger commented Nov 29, 2023

Hi!
I have tried coding something for this. Check the following:

I added a new method parameter_info to provide information about different parameter types. This method is intended to enhance the functionality by offering insights into the parameters used in the code.

    def parameter_info(self, param_class, param_type):
        parameter_info = ""
        parameters = self._find_symbols(param_class)
        for parameter in parameters:
            if isinstance(parameter, pybamm.FunctionParameter):
                if parameter.name not in parameter_info:
                    input_names = "'" + "', '".join(parameter.input_names) + "'"
                    parameter_info += (
                        f"{parameter.name} ({param_type} with input(s) {input_names})\n"
                    )

            elif isinstance(parameter, pybamm.InputParameter):
                if not parameter.domain:
                    parameter_info += f"{parameter.name} ({param_type})\n"
                else:
                    parameter_info += (
                        f"{parameter.name} ({param_type} in {parameter.domain})\n"
                    )

            elif isinstance(parameter, pybamm.Parameter):
                parameter_info += f"{parameter.name} ({param_type})\n"
        return parameter_info

    def print_parameter_info(self):
        self._parameter_info = ""
        parameter_types = [
            ("Parameter", pybamm.Parameter),
            ("inputParameter", pybamm.InputParameter),
            ("FunctionParameter", pybamm.FunctionParameter),
        ]

        for param_type, param_class in parameter_types:
            parameter_info = self.parameter_info(param_class, param_type)
            self._parameter_info += parameter_info

        print(self._parameter_info)

The code works as intended. It also passed the pre-commit checks. If there are any suggestions for improvement or if additional modifications are needed, please let me know. I'm open to making any necessary revisions.

NOTE: This would be my first contribution (both to PyBaMM and an organisation in general) so I would appreciate feedback or suggestions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours feature priority: medium To be resolved if time allows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants