Repository: pybamm-team/PyBaMM (python)

Issue:
Use of constant parameter functions throws an error
If one uses any model (inbuilt or self-made) with a function parameter that doesn't process any of the variables, its output won't be a Symbol, but a float.

Steps ro reproduce the error most easily:
1. Load up the example notebook/script for e.g. the DFN model.
2. Add/Update the following parameter in the default parameter set:
    "Electrolyte diffusivity [m2.s-1]": lambda c_e, T, T_ref, E_D_e, R: 2.72e-10,
3. Run the notebook/script.
4. See beautiful error message at the end. It basically fails at the step where 2.72e-10 obviously has no ".id" field, since it's not a Symbol.

While this is easily fixed by multiplying the 2.72e-10 by (0 * T_ref + 1) or replacing it with pybamm.Scalar(2.72e-10), I think this might be a common issue for beginners when using a model with more complexity than needed. Thus I'd like a helpful error message that hints me at my error. Or even better: constant parameter functions get converted to an appropriate Symbol automatically, just like it happens with scalar parameters already.

I monkey-patched the following hint-giver in my local copy (0.2.0 pip install) for my purposes:
in file pybamm/parameters/parameter_values.py, after line 472:
        except AttributeError:
            raise ValueError("A parameter was not given as a 'Symbol': " + repr(symbol)) from None
This displays "A parameter was not given as a 'Symbol': 2.72e-10" in the example case. I didn't figure out how to get the parameter name in this context.



Beautiful error message:

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 467, in process_symbol
    return self._processed_symbols[symbol.id]
KeyError: -1310443411252241803

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 467, in process_symbol
    return self._processed_symbols[symbol.id]
KeyError: 6247489124244137513

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 467, in process_symbol
    return self._processed_symbols[symbol.id]
KeyError: -5616125575659663777

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 467, in process_symbol
    return self._processed_symbols[symbol.id]
KeyError: -9191736092148201685

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 467, in process_symbol
    return self._processed_symbols[symbol.id]
KeyError: -8136991043322325845

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 467, in process_symbol
    return self._processed_symbols[symbol.id]
KeyError: -5859718582323950464

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "solve.py", line 86, in <module>
    parameters.process_model(model)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 350, in process_model
    model.rhs[variable] = self.process_symbol(equation)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 469, in process_symbol
    processed_symbol = self._process_symbol(symbol)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 531, in _process_symbol
    new_child = self.process_symbol(symbol.child)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 469, in process_symbol
    processed_symbol = self._process_symbol(symbol)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 531, in _process_symbol
    new_child = self.process_symbol(symbol.child)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 469, in process_symbol
    processed_symbol = self._process_symbol(symbol)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 522, in _process_symbol
    new_left = self.process_symbol(symbol.left)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 469, in process_symbol
    processed_symbol = self._process_symbol(symbol)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 522, in _process_symbol
    new_left = self.process_symbol(symbol.left)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 469, in process_symbol
    processed_symbol = self._process_symbol(symbol)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 523, in _process_symbol
    new_right = self.process_symbol(symbol.right)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 469, in process_symbol
    processed_symbol = self._process_symbol(symbol)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 518, in _process_symbol
    return self.process_symbol(function_out)
  File "/usr/local/lib/python3.7/dist-packages/pybamm/parameters/parameter_values.py", line 467, in process_symbol
    return self._processed_symbols[symbol.id]
AttributeError: 'float' object has no attribute 'id'


Pull request:
#917 fix function that returns float
# Description

Fixes #917 

## Type of change

Please add a line in the relevant section of [CHANGELOG.md](https://github.com/pybamm-team/PyBaMM/blob/master/CHANGELOG.md) to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

- [ ] New feature (non-breaking change which adds functionality)
- [ ] Optimization (back-end change that speeds up the code)
- [x] Bug fix (non-breaking change which fixes an issue)


# Key checklist:

- [x] No style issues: `$ flake8`
- [x] All tests pass: `$ python run-tests.py --unit`
- [x] The documentation builds: `$ cd docs` and then `$ make clean; make html`

You can run all three at once, using `$ python run-tests.py --quick`.

## Further checks:

- [x] Code is commented, particularly in hard-to-understand areas
- [x] Tests added that prove fix is effective or that feature works


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d40f40642..6e455c961 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@
 
 ## Bug fixes
 
+-   Fixed bug raised if function returns a scalar ([#919](https://github.com/pybamm-team/PyBaMM/pull/919))
 -   Updated Getting started notebook 2 ([#903](https://github.com/pybamm-team/PyBaMM/pull/903))
 -   Reformatted external circuit submodels ([#879](https://github.com/pybamm-team/PyBaMM/pull/879))
 -   Some bug fixes to generalize specifying models that aren't battery models, see [#846](https://github.com/pybamm-team/PyBaMM/issues/846)
diff --git a/pybamm/parameters/parameter_values.py b/pybamm/parameters/parameter_values.py
index 81a6c6861..7b2b1756b 100644
--- a/pybamm/parameters/parameter_values.py
+++ b/pybamm/parameters/parameter_values.py
@@ -524,6 +524,9 @@ class ParameterValues:
                 # return differentiated function
                 new_diff_variable = self.process_symbol(symbol.diff_variable)
                 function_out = function.diff(new_diff_variable)
+            # Convert possible float output to a pybamm scalar
+            if isinstance(function_out, numbers.Number):
+                return pybamm.Scalar(function_out)
             # Process again just to be sure
             return self.process_symbol(function_out)
 


Test patch:
diff --git a/tests/unit/test_parameters/test_parameter_values.py b/tests/unit/test_parameters/test_parameter_values.py
index 30a75ddd4..c9d81797d 100644
--- a/tests/unit/test_parameters/test_parameter_values.py
+++ b/tests/unit/test_parameters/test_parameter_values.py
@@ -300,6 +300,7 @@ class TestParameterValues(unittest.TestCase):
                 "a": 3,
                 "func": pybamm.load_function("process_symbol_test_function.py"),
                 "const": 254,
+                "float_func": lambda x: 42,
             }
         )
         a = pybamm.InputParameter("a")
@@ -320,6 +321,11 @@ class TestParameterValues(unittest.TestCase):
         processed_diff_func = parameter_values.process_symbol(diff_func)
         self.assertEqual(processed_diff_func.evaluate(u={"a": 3}), 123)
 
+        # function parameter that returns a python float
+        func = pybamm.FunctionParameter("float_func", a)
+        processed_func = parameter_values.process_symbol(func)
+        self.assertEqual(processed_func.evaluate(), 42)
+
         # function itself as input (different to the variable being an input)
         parameter_values = pybamm.ParameterValues({"func": "[input]"})
         a = pybamm.Scalar(3)