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

Min model error #28

Merged
merged 41 commits into from
Jan 29, 2024
Merged

Min model error #28

merged 41 commits into from
Jan 29, 2024

Conversation

brendan-m-murphy
Copy link
Contributor

@brendan-m-murphy brendan-m-murphy commented Dec 5, 2023

Changes:

  • added an option for passing arbitrary keyword arguments to run_hbmcmc.py,
  • added kwargs to fixedbasisMCMC, so that additional keyword args can be used (currently they are just passed to inferpymc)
  • added min_error as an argument to inferpymc; the default value is 0.0, so it has no effect, but it can be specified in the ini file or via the command line.

Syntax for run_hbmcmc.py:

python run_hbmcmc.py -c example.ini --kwargs='{"outputpath": "/user/home/ab12345", "min_error": 20.0}'

In this case, the outputpath argument in example.ini would be replaced with "/user/home/ab12345", and the argument min_error=20.0 would be passed to inferpymc.

Note that the value passed to --kwargs is a "dictionary in string form". Use single quotes for the outside of the string, and use double quotes for the keys inside the curly braces, as well as for any values that are strings. The argument is parsed by json.loads, which will convert a json in string format to a dictionary. (Note: do not put integer or float keyword values in double quotes, unless you want them to be parsed as strings.)

EricSaboya and others added 30 commits October 31, 2023 14:38
added option to save or reload a merged data object
If `filters = []` the previous version threw an
error because it tries to apply string methods to `None`.

This is just a quick fix.
This allow arbitrary keyword args to be based to the inversion
function. For instance:

`python run_hbmcmc.py -c example.ini --kwargs="{'outputpath':
'/user/work/ab12345'}"`
will override the output path specified in `example.ini`.
This can be passed to `fixedbasisMCMC`, via
a .ini file, or as a command line argument (using the
kwargs feature added in a previous commit.)
This allows merged data to be used without having access
to the object store used to create the merged data.

NOTE: currently this just takes the first `FluxData` object
in the `fp_all['.flux']` dictionary.
cv18710 added 4 commits January 25, 2024 14:41
…post mean country fluxes and create shared netCDF flux files
…orrect prior flux month is found. This includes correctly selecting prior fluxes from annual fluxes and multi-month inversions
brendan-m-murphy and others added 6 commits January 26, 2024 17:22
Updates to min_model_error, to bring it up to date with changes in eric_devel etc.
`flux` is no longer needed as a parameter in `inferpymc_postprocess` due
to changes recently added by Alice.
Copy link
Collaborator

@EricSaboya EricSaboya 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 to me.

@brendan-m-murphy brendan-m-murphy merged commit 4b30cb1 into main Jan 29, 2024
1 check passed
@brendan-m-murphy brendan-m-murphy deleted the min_model_error branch January 29, 2024 17:43
@aliceramsden
Copy link
Collaborator

Hi @brendan-m-murphy, I'm getting an error in get_data when it tries to read in obs when there isn't any available that month. It gets to line 153 in get_data (unit = float(site_data[site].mf.units) and fails because site_data is None. I think the try/except loop isn't catching this because it's a TypeError instead of a SearchError, maybe? (I'm not very confident about how error messages work in this case!) Is there any way of setting that try/except loop to work with either type of error in get_data?

@brendan-m-murphy
Copy link
Contributor Author

brendan-m-murphy commented Jan 30, 2024

Hi @aliceramsden , I guess it depends what we want to happen in this case. If we already have a value for units from a previous site, then it makes sense to keep going, but if we don't have a value for units already, the step that adds BC will fail I think.

Another issue is that I thought get_obs_surface never actually returns None unless you're using OpenGHG "on the cloud", which isn't a mode we actually use.

Could you tell me start/end date, species, and sites? If the species is fixed, we only need to get the units once. (I guess it would be best if we could specify it ahead of time so it doesn't need to be inferred.)

@brendan-m-murphy
Copy link
Contributor Author

Sorry I just realised that skipping would sort this problem in the short term. Although I don't think it's the ideal behaviour, since either we know the units from some site, or none of the sites will be used and we'll have no data for that inversion period.

@brendan-m-murphy
Copy link
Contributor Author

I think the try/except loop isn't catching this because it's a TypeError instead of a SearchError, maybe? (I'm not very confident about how error messages work in this case!) Is there any way of setting that try/except loop to work with either type of error in get_data?

You're right about this: the except clause is only catching SearchErrors. The reason for narrowing the type of exceptions to catch is that I was only expecting SearchError to happen if data isn't found, but it looks like we might get None in some cases... which should probably be fixed in openghg...

@brendan-m-murphy
Copy link
Contributor Author

So apparently get_obs_surface will return None if the data you retrieve has a time coordinate, but the time coordinate is empty. So I guess we just want to skip this case.

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.

3 participants