PyBaMM icon indicating copy to clipboard operation
PyBaMM copied to clipboard

Use `hypothesis` to implement property based testing.

Open prady0t opened this issue 1 year ago • 11 comments

Pybamm migrated from unittest to pytest successfully in the last GSoC cohort. See : https://pybamm.org/gsoc/2024/#migrate-from-unittest-to-pytest-and-improve-pybamms-testing-infrastructure . What now remains is implementing hypothesis for property based testing. This will ensure more robust tests and will help us find edge cases and bugs more easily. Here's an example of how we can write tests using hypothesis : https://hypothesis.works/articles/hypothesis-pytest-fixtures/ The goal here would be to identify all the test function that can benefit from property based testing and later using @given decorator to give that test function set of properties.

Once we have enough tests we can use tools such as oss-fuzz to run it continuously on the cloud. See : https://google.github.io/oss-fuzz/getting-started/new-project-guide/python-lang/#hypothesis

prady0t avatar Dec 21 '24 18:12 prady0t

Hey it looks like a interesting issue, i am willing to work on it @prady0t @agriyakhetarpal

RohitP2005 avatar Dec 22 '24 15:12 RohitP2005

Sure! Please go ahead. Let us know if you need any help. I'll try to find out some tests where hypothesis can be used, it will help you get started. Later you can find more such tests on your own.

prady0t avatar Dec 22 '24 15:12 prady0t

Thanks i will start my work on it @prady0t

RohitP2005 avatar Dec 22 '24 15:12 RohitP2005

hey @prady0t I thougth maybe I should go by folder inside the /tests/unit to find test that can benefits from the property based testing and integrate them using hypothesis Can u give a place to start

RohitP2005 avatar Dec 26 '24 20:12 RohitP2005

Hi @RohitP2005, sorry for the late response. Yes you can start by looking into unit tests first. Here's a file where you can start

https://github.com/pybamm-team/PyBaMM/blob/a1f73b6780187e9635c0c4a1164531cbf73fe70c/tests/unit/test_expression_tree/test_averages.py#L11

This file has tests to calculate averages and input-output can be bounded by properties. For a quick example, here's how you can modify the above test :

Image

This is a very basic example but demonstrates the use case quite well. What I suggest personally is to make a list of all the test files and start going through each one, implementing hypothesis wherever you can. You can keep checking out the boxes for every file you visited. In this way you will end up going through all the tests and others can also keep track of the progress, otherwise it will be a huge mess. I did something similar while migrating from unittest to pytest see #4156.

Here's another example, this class also has a lot of mathematical functions

https://github.com/pybamm-team/PyBaMM/blob/a1f73b6780187e9635c0c4a1164531cbf73fe70c/tests/unit/test_expression_tree/test_unary_operators.py#L16C1-L16C26

Try to find out clever ways you can use hypothesis in these tests, later you can open up a tracking issue and start working on other files.

Here's some reading material that could be helpful :

  • https://pytest-with-eric.com/pytest-advanced/hypothesis-testing-python/
  • https://www.lambdatest.com/blog/hypothesis-testing-in-python/
  • https://hypothesis.readthedocs.io/en/latest/data.html

prady0t avatar Dec 27 '24 13:12 prady0t

Before opening a tracking issue I would suggest going through the above mentioned tests and opening a PR covering both files first. We do not want to flood maintainers with a lot of PRs so it would be great if we group a lot of files together and open PRs when we have a lot of changes (just enough so that they are not difficult to review). Also remember not all tests can be tested like this, in fact the once that I mentioned above might not be the best examples.

The goal would be to first identify and list all files that needs such a change, do a lot of changes locally and then open a PR. The checklist looks fine, just remove the init.py files.

Would love @agriyakhetarpal @arjxn-py @Saransh-cpp thoughts on this.

prady0t avatar Dec 27 '24 14:12 prady0t

Before opening a tracking issue I would suggest going through the above mentioned tests and opening a PR covering both files first. We do not want to flood maintainers with a lot of PRs so it would be great if we group a lot of files together and open PRs when we have a lot of changes (just enough so that they are not difficult to review). Also remember not all tests can be tested like this, in fact the once that I mentioned above might not be the best examples.

The goal would be to first identify and list all files that needs such a change, do a lot of changes locally and then open a PR. The checklist looks fine, just remove the init.py files.

Would love @agriyakhetarpal @arjxn-py @Saransh-cpp thoughts on this.

understood , this checklist just shows the available files inside the unit dir, later on reviewing and identifying files which require changes we can remove unecessary files from the checklist . As u requested i will work on those two files first and open a PR , later we can work on the rest

RohitP2005 avatar Dec 27 '24 15:12 RohitP2005

One thing to keep in mind is that hypothesis is like a engine for a car, and it takes a very long time to generate its adversarial examples, both when starting the test suite at test collection time, and then when running the tests – and our combination of unit and integration tests already takes thirty minutes in CI. Hypothesis will only bump this number up, so it should be used sparingly and where it is justified to do so, like @prady0t has mentioned. For example, a test file that covers utility files where the functionality to be tested is not numerical in nature will not benefit from property-based testing and we should exclude it, it won't be worth it. Other strategies to reduce our testing time are especially welcome.

agriyakhetarpal avatar Dec 27 '24 22:12 agriyakhetarpal

That covers test_unary_operators.py and test_averages.py. @prady0t Any suggestion on how to proceed next I think we shall implement it category wise, eg: we should cover the /test_expression_tree/ directory

RohitP2005 avatar Feb 25 '25 03:02 RohitP2005

@RohitP2005 I think we should first make a list of tests that can benefit most from this and will make sense to run in CI jobs given the extra time and resources it takes.

prady0t avatar Mar 03 '25 23:03 prady0t

@prady0t Sounds Good ! I will prepare such list of tests , any idea on how to categorize them for organization

RohitP2005 avatar Mar 04 '25 16:03 RohitP2005