search

openghg / openghg_inversions

University of Bristol Atmospheric Chemistry Research Group RHIME Inversion code (with openghg dependency)
MIT License
5 stars 0 forks source link

Added option to use obs to compute pollution event size #111

Closed brendan-m-murphy closed 1 month ago

brendan-m-murphy commented 2 months ago

Using the option pollution_events_from_obs = True uses the obs minus the modelled baseline as the size of the pollution events in the model-data mismatch error. (If use_bc is False, then just the obs are used as the pollution event size.)

This addresses issue #110

ag12733 commented 2 months ago

I think the issue is just lack of documentation. I don’t think it states that to use use_bc=False means that you need to pre-subtract the bc from the data so that y (obs) represents only the enhancements.

I think Joe’s point is because he didn’t know that obs are only enhancements so there are no fundamental issues, just lack of documentation I think!

Dr. Anita Ganesan Associate Professor School of Geographical Sciences University of Bristol Bristol, BS8 1SS, United Kingdom Tel: +44 117 455 1545<tel:+441174551545> Email: @.**@.>

From: Joe Pitt @.> Date: Wednesday, 1 May 2024 at 21:00 To: openghg/openghg_inversions @.> Cc: Anita Ganesan @.>, Review requested @.> Subject: Re: [openghg/openghg_inversions] Added option to use obs to compute pollution event size (PR #111)

@joe-pitt commented on this pull request.

In general this looks good to me. The only thing I'm not sure about is the case where use_bc=False. There may be some instances where we're happy to apply sig to the absolute mole fraction, but clearly there are other cases where this would not be sensible. Do we want to print some sort of warning here? I'm not sure whether it would be intuitive to people what would happen with usebc=False, pollution_events_from_obs=True

— Reply to this email directly, view it on GitHubhttps://github.com/openghg/openghg_inversions/pull/111#pullrequestreview-2034349811, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AVCIICGHOCRXM3EEP5XASVDZAFCT7AVCNFSM6AAAAABHCCYIDOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMZUGM2DSOBRGE. You are receiving this because your review was requested.Message ID: @.***>

brendan-m-murphy commented 2 months ago

There is some extra documentation about these extra parameters in the numpyro PR, I was planning to document these changes once this gets merged into devel, because I could pull these changes into the PR with the new documentation.

The docstring should have been updated though, maybe I forgot to do that.