Numpyro sampler option, documentation, land/sea map fix, save trace fix #101
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I *think* tensorflow probability is needed to get truncated normal distributions in Numpyro, but I can't find a reference for this now.
The html wasn't rendering properly on github.
brendan-m-murphy
changed the title
This name can be passed to `fixedbasisMCMC`, and then passed on to `data_processing_surface_notracer`, or it will be created based on the formula in `get_data.py`
- `skip_postprocess` will be used for profiling runs - `merged_data_only` can be used to merge data ahead of time; this will also be used for profiling performance
PBLH is being replaced by atmosphere_boundary_layer_thickness in OpenGHG v0.8. Also, the new merged data just stores "inlet", whereas old merged data also stored "inlet_height_magl" and "height".
`min_model_error` was also added to the attributes of the output dataset, so that the calculated value can be recovered. The `residual_error_method` function should probably be moved from `hbmcmc.py` once there is a location for other preprocessing functions.
Any NaNs after this point will cause errors in pymc.
brendan-m-murphy
requested review from
aliceramsden,
joe-pitt,
ag12733,
qq23840,
pearson01,
hdelongueville and
alexdanjou
|
@joe-pitt reports: error using weighted basis with fixed outer regions: [Yesterday 21:23] Joe Pitt [Yesterday 21:24] Joe Pitt |
joe-pitt
approved these changes
May 3, 2024
There was a problem hiding this comment.
It turns out the error Brendan mentioned only occurs for a certain flux file (or potentially a certain type of flux file), and it is actually giving the same behaviour on devel as on this branch. So it isn't an issue with these changes. I have successfully run a test ch4 inversion on this branch so I'm going to approve the PR.
This was referenced May 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch adds the option to use Numpyro's NUTS sampler in place of the pymc NUTS sampler. This seems to be 7-10x faster. This addresses issue #17 and the ACRG project issue #30, although profiling needs to be done to assess the speed improvements.
It also adds some documentation by expanding the readme file and adding a short overview (https://github.com/openghg/openghg_inversions/tree/numpyro-fixed-outer-regions)
There are fixes to two other issues as well:
save_traceoption was removed in a previous PR; it is restored in this PR.Also, a link to the DOI associated with releases was added to the readme file, and a test was added for the bucket/weighted basis function.