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

Added full inversion test with all required data #25

Merged
merged 9 commits into from
Oct 5, 2023

Conversation

brendan-m-murphy
Copy link
Contributor

@brendan-m-murphy brendan-m-murphy commented Sep 15, 2023

This is a test that runs fixedbasisMCMC for one iteration with 4 basis functions, over two days of data. It runs in about 20 seconds, so it should be useful for checking that minor changes to the code don't break existing functionality.

This test doesn't cover the run_hbmcmc script. The changes to hbmcmc.py are mostly due to black; there are a few changes to make it easier to choose where the quadtree basis is saved.

I added a test with more iterations, which is marked "slow". I set up pytest.ini to not run "slow" tests by default. The slow test uses a week of data, 50 basis functions, and 5000 iterations; it runs in about 7-8 minutes on my macbook.

In the future, I may add more "slow" tests that are marked "blue_pebble", which will only run on blue pebble, for tuning purposes. (Alternatively, this could be done with a separate script.)

For set-up, conftest.py creates an object in a temporary directory using some data files distributed with openghg_inversions. If this object store already exists and has data in it, then no data is standardised, which should speed up subsequent tests. (This is also a test_conftest.py file that checks if the right data is available.) Some data needed to run the inversion test isn't stored in the object store; this data is added to "bc_basis_functions" and "countries" by conftest when the tests are first run (if they don't exist, conftest creates these directories.)

To keep the openghg_inversions repo clean while running tests, I changed how quadtreebasis works: if "outputdir" is passed, then the basis is saved there, rather than in a temporary directory (which with the current code, would be created where the tests are run from). I also added some safe guards to the part of hbmcmc.py that deletes the temporary basis file. Saving the quadtree to the output folder is controlled by a flag in fixedbasisMCMC, so it doesn't change the behaviour of the code unless you ask it to.

Closes issue #8.

@brendan-m-murphy brendan-m-murphy marked this pull request as ready for review September 15, 2023 13:35
I registered a "slow" mark in pytest.ini and
skip tests with this tag by default.

The test added runs an inversion for a week of data, with 5000
iterations.

I'm not sure how useful it is now, although if this test runs quickly,
then there is probably something wrong with the model.
I changed it temporarily to help debug something and forgot
to change it back.
Now these are created automatically by conftest.

Checks are made to avoid overwriting any files already
present in these directories.

In the future, these files will be managed by something like
the object store.
Copy link
Contributor

@gareth-j gareth-j left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@brendan-m-murphy brendan-m-murphy merged commit 6858aee into main Oct 5, 2023
1 check passed
@brendan-m-murphy brendan-m-murphy deleted the adding_tests branch October 5, 2023 08:12
@gareth-j gareth-j mentioned this pull request Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants