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

Mean, Normalizer, Trend and Fitting #124

Merged
merged 100 commits into from
Mar 13, 2021
Merged

Mean, Normalizer, Trend and Fitting #124

merged 100 commits into from
Mar 13, 2021

Conversation

MuellerSeb
Copy link
Member

@MuellerSeb MuellerSeb commented Dec 21, 2020

Closes: #94

Abstract

This PR adds:

  • a new submodule containing normalizer classes.
  • a callable mean (added to normalized data) for all Field classes
  • a callable trend (added to again-transformed / input data) for all Field classes
  • normalizer as input for all field classes
  • auto fit option in Krige class (normalizer and variogram)

Purpose of Normalizer

Their purpose is to normalize (follow a normal distribution) and denormalize data.
Since Krige and SRF produce normal distributed fields, these normalizers offer the opportunity to transform fields into a desired distribution and, in case of krige, to normalize conditioning data beforehand.

Normalizer

  • LogNormal
  • BoxCox
  • BoxCoxShift
  • YeoJohnson
  • Modulus
  • Manly

Normalizer Fitting

In order to fit hyper-parameter of the normalization transformations, the Normalize base-class provides a fit method, that estimates parameters by maximizing the log-likelihood function for given data.

Callable trend and mean

In order to use trends and variable means from external source to play nicely together with Normalizer, we will allow callable means to be added to normalized data and trends applied to input data.

Auto Fitting for Krige

SInce we provide an automated workflow for estimating the empirical variogram with automatic binning, we plug that into the kriging class as an automated workflow. In addition the normalizer can be fitted automatically as well by minimizing the likelihood function as described above.

Affected Routines

  • new method Field.post_field (replacing Krige._post_field and is similar to Field.pre_pos)
  • Krige._krige_cond needs to care about normalizer and callable mean in correct order
  • Field.__init__ needs trend_function, normalizer and fit_normalizer arguments
  • add Krige.cond_mean holding mean values at conditioning points

TODO

  • add to all Fields as keywords
  • add examples
  • add tests

@MuellerSeb MuellerSeb added the enhancement New feature or request label Dec 21, 2020
@MuellerSeb MuellerSeb added this to the 1.3 milestone Dec 21, 2020
@MuellerSeb MuellerSeb self-assigned this Dec 21, 2020
@MuellerSeb MuellerSeb linked an issue Dec 21, 2020 that may be closed by this pull request
@MuellerSeb MuellerSeb marked this pull request as draft December 21, 2020 19:42
@MuellerSeb MuellerSeb changed the title Normalizer Normalizer and Trend Jan 13, 2021
@MuellerSeb MuellerSeb marked this pull request as ready for review February 22, 2021 20:24
@MuellerSeb MuellerSeb removed the request for review from fhesze February 22, 2021 20:25
@MuellerSeb
Copy link
Member Author

@LSchueler this is finally ready for review. 😉 🎉

gstools/krige/base.py Show resolved Hide resolved
gstools/normalizer/tools.py Show resolved Hide resolved
Copy link
Member

@LSchueler LSchueler left a comment

Choose a reason for hiding this comment

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

Wow, what a tremendous piece of work... again ;-)

Unfortunately, I won't be able to finish this review today. But in case you'd like to here some criticism already, here it goes:

I don't like the argument only_mean in Krige.__call__(). I think that should be taken care of in a separate method. I'm afraid the __call__ method could get as cluttered as SRF.__call__() was before you cleaned it up.

@MuellerSeb
Copy link
Member Author

I don't like the argument only_mean in Krige.__call__(). I think that should be taken care of in a separate method. I'm afraid the __call__ method could get as cluttered as SRF.__call__() was before you cleaned it up.

I also struggled with that one, but:

  1. This routine whould have the exact same signature as __call__()
  2. When using a separate method, we can't use structured, unstructured or mesh with it
  3. The argument could have another/a better name, though. "Officially" this procedure is called "kriging the mean". Maybe we could shorten it from only_mean to mean?

A separate routine would have more downsides IMHO.

Copy link
Member

@LSchueler LSchueler left a comment

Choose a reason for hiding this comment

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

This is soo cool! - I'm really sorry I kept you waiting for so long for this review.

Some comments in the code and apart from that I'm still not convinced by the only_mean argument in the Krige.__call__() method. The structured and unstructured methods are only syntactic sugar and maybe a method Krige.the_mean or Krige.krige_the_mean might be much clearer.
But I wouldn't try to block this PR if you disagree ;-)

examples/03_variogram/06_auto_bin_latlon.py Show resolved Hide resolved
examples/09_spatio_temporal/02_precip_2d.py Show resolved Hide resolved
examples/10_normalizer/01_auto_fit.py Show resolved Hide resolved
examples/10_normalizer/01_auto_fit.py Show resolved Hide resolved
gstools/field/tools.py Outdated Show resolved Hide resolved
gstools/field/tools.py Outdated Show resolved Hide resolved
gstools/normalizer/base.py Show resolved Hide resolved
gstools/normalizer/methods.py Show resolved Hide resolved
gstools/normalizer/methods.py Show resolved Hide resolved
@LSchueler
Copy link
Member

Fixed some typos on this branch, don't forget to pull.

@MuellerSeb
Copy link
Member Author

This is soo cool! - I'm really sorry I kept you waiting for so long for this review.

Some comments in the code and apart from that I'm still not convinced by the only_mean argument in the Krige.__call__() method. The structured and unstructured methods are only syntactic sugar and maybe a method Krige.the_mean or Krige.krige_the_mean might be much clearer.
But I wouldn't try to block this PR if you disagree ;-)

I'd like to keep it this way, since also mesh is using the call function and all future interfaces as well.

@MuellerSeb MuellerSeb merged commit d240cec into develop Mar 13, 2021
@MuellerSeb
Copy link
Member Author

@LSchueler thanks for the review. I updated everything. Merging this beast now! 🎉

@MuellerSeb MuellerSeb deleted the normalizer branch March 13, 2021 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] add normalization and trend to Field
2 participants