Make parameters importable

[priyanshuone6]

Description

• Modify load_function as it imports the function without the complete module path and that causes issues while unpickling. It also removes the (incomplete) module path from sys.modules to avoid namespace clashes.
• Edit tests
• Add __init__.py in parameters to convert them to modules and make them importable through pybamm

Fixes #960

Type of change

Please add a line in the relevant section of 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)
• Bug fix (non-breaking change which fixes an issue)

Key checklist:

• No style issues: $ flake8
• All tests pass: $ python run-tests.py --unit
• 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:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[codecov]

Codecov Report

Merging #1475 (59a2fdf) into develop (b4735e1) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

❗ Current head 59a2fdf differs from pull request most recent head 06f4ec9. Consider uploading reports for the commit 06f4ec9 to get more accurate results

@@             Coverage Diff             @@
##           develop    #1475      +/-   ##
===========================================
- Coverage    98.37%   98.30%   -0.07%     
===========================================
  Files          282      295      +13     
  Lines        16602    16742     +140     
===========================================
+ Hits         16332    16459     +127     
- Misses         270      283      +13     

Impacted Files	Coverage Δ
...2019/electrolyte_TDF_EC_DMC_1_1_Landesfeind2019.py	100.00% <100.00%> (ø)
...trolyte_conductivity_EC_DMC_1_1_Landesfeind2019.py	100.00% <100.00%> (ø)
...ctrolyte_diffusivity_EC_DMC_1_1_Landesfeind2019.py	100.00% <100.00%> (ø)
..._transference_number_EC_DMC_1_1_Landesfeind2019.py	100.00% <100.00%> (ø)
...2019/electrolyte_TDF_EC_EMC_3_7_Landesfeind2019.py	100.00% <100.00%> (ø)
...trolyte_conductivity_EC_EMC_3_7_Landesfeind2019.py	100.00% <100.00%> (ø)
...ctrolyte_diffusivity_EC_EMC_3_7_Landesfeind2019.py	100.00% <100.00%> (ø)
..._transference_number_EC_EMC_3_7_Landesfeind2019.py	100.00% <100.00%> (ø)
...19/electrolyte_TDF_EMC_FEC_19_1_Landesfeind2019.py	100.00% <100.00%> (ø)
...olyte_conductivity_EMC_FEC_19_1_Landesfeind2019.py	100.00% <100.00%> (ø)
... and 19 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4735e1...06f4ec9. Read the comment docs.

[valentinsulzer]

Thanks @priyanshuone6 , changes look good. Is there a way to test that saving and loading a sim now works in the case it didn't before?

[tlestang]

Maybe worth adding a comment explaining what this does and why?
Is it because you want to strip the full path to the module from ../PyBaMM/pybamm/input/example.py to pybamm/input/example.py ?

[priyanshuone6]

Yes, I'll add a comment

[tlestang]

So that works if pybamm is imported from a directory named PyBaMM (which is indeed default when cloning from github). But what happens if pybamm is imported from its installed location (e.g. ../site-packages/pybamm)? This would be the case when installing PyBaMM with pip instead of importing it directly from the sources

[tlestang]

You committed the pickle file test_load_param.p but we don't really know what it corresponds to. How about, instead, this tests saves a simulation before trying to load it?

[priyanshuone6]

@tlestang The bug is reproducible only when the pickle file is loaded afresh in a new terminal. If we'll save and then load it then the parameter will already be imported and the error won't occur.

[tlestang]

Ah yes. Alternatively we could write the pickle from another process, something like

save_sim = "import pybamm; model = pybamm.lithium_ion.SPM(); pybamm.Simulation(model).save("test_load_param.p")"
subprocess.run("python -c {save_sim}")

Relying on the binary pickle only may make debugging hard in the future, when nobody remembers what is contains :)
Which makes me think another option is adding a comment that describes how you generated the pickle file.

[priyanshuone6]

@tlestang Thank you for all the suggestions, I have added the subprocess.run in the test and also tested it with the develop branch and it worked.

[tlestang]

Looks good @priyanshuone6 , nice one :)