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

Add sum model #364

Open
wants to merge 57 commits into
base: main
Choose a base branch
from
Open

Add sum model #364

wants to merge 57 commits into from

Conversation

MuellerSeb
Copy link
Member

@MuellerSeb MuellerSeb commented Aug 11, 2024

This PR adds a SumModel class to represent sums of Covariance models.

Sum-Model

  • added SumModel class
    • represents sum of covariance models
    • behaves just as a normal covariance model with kriging and field generation
    • covariance models can be added with overloaded + operator: model = m1 + m2
    • class is subscriptable to access sub-models by index: m1 = model[0]
    • included models will get a nugget of 0 and the nugget is stored separately in the sum-model
    • model variance is the sum of the sub-model variances
    • model length-scale is weighted sum of sub-model len-scales, where the weights are the ratios of the sub-models variance to the sum variance (motivated by the integral scale, which satisfies this relation)
    • anisotropy and rotation need to be the same for all included sub-models
    • parameters of the sub-models can be accessed by name with added index suffix: model[0].nu == model.nu_0
    • fitting: if len_scale is fixed, none of the len_scale_<i> can be fixed since len_scale is calculated from variance ratios
  • added Nugget class (empty SumModel)
    • allow len scale of 0 in CovModel to enable a pure nugget model
    • added zero_var and model attributes to Generator ABC to shortcut field generation for pure nugget models
import gstools as gs
m1 = gs.Gaussian(dim=2, var=1.0, len_scale=2.0)
m2 = gs.Gaussian(dim=2, var=2.0, len_scale=20.0)
model = m1 + m2
model.plot()

Other changes

  • removed var_raw attribute from CovModel (was rarely used and only relevant for the truncated power law models)
    • BREAKING CHANGE (but not to many should be affected)
    • TPLCovModel now has a intensity attribute which calculates what var_raw was before
  • simplified variogram fitting (var_raw was a bad idea in the first place)
  • variogram plotting now handles a len-scale of 0 (to properly plot nugget models)
  • fitting: when sill is given and var and nugget are deselected from fitting, an error is raised if given var+nugget is not equal to sill (before, they were reset under the hood in a strange way)

@MuellerSeb MuellerSeb marked this pull request as draft August 11, 2024 15:05
@MuellerSeb MuellerSeb added this to the 1.7 milestone Feb 3, 2025
@MuellerSeb MuellerSeb added enhancement New feature or request Refactoring Code-Refactoring needed here labels Feb 3, 2025
@MuellerSeb MuellerSeb self-assigned this Feb 3, 2025
@MuellerSeb MuellerSeb marked this pull request as ready for review February 8, 2025 13:11
@MuellerSeb MuellerSeb requested a review from LSchueler February 8, 2025 13:11
@MuellerSeb
Copy link
Member Author

@LSchueler this is ready for review.

@LSchueler
Copy link
Member

Awesome, I'll need some time to thoroughly go throw your changes.
But I already saw that you claimed example number 11 for the sum model after I did the same for the plurigaussian fields :-D Do we just keep adding new examples in an ascending order according to the time a PR is merged?

@MuellerSeb
Copy link
Member Author

First come, first serve :-D

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.

Sorry for taking so long to finish this review, but as you know, I had the best reasons ;-)

I'm really looking forward to sum models being integrated into GSTools, but I found quite a few things which should be addressed before merging.

We'll combine a Spherical and a Gaussian covariance model to construct
a sum model, visualize its variogram, and generate spatial random fields.

Let's start with importing GSTools setting up the domain size.
Copy link
Member

Choose a reason for hiding this comment

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

"and" missing

Comment on lines +17 to +20
# First, we create two individual covariance models: a :any:`Spherical` model and a
# :any:`Gaussian` model. The Spherical model will emphasize small-scale variability,
# while the Gaussian model captures larger-scale patterns.

Copy link
Member

Choose a reason for hiding this comment

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

This formulation might give the impression that the small-scale and large-scale patterns are due to the covariance models. You should clarify that this is due to the chosen length scales.

Comment on lines +34 to +35
# As shown, the Spherical model controls the behavior at shorter distances,
# while the Gaussian model dominates at longer distances.
Copy link
Member

Choose a reason for hiding this comment

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

I think you should also briefly mention how the variance of the covariance models influences the resulting fields.


###############################################################################
# We fit the sum model to the empirical variogram using GSTools' built-in
# fitting capabilities. As seen in the representation, the variances and length
Copy link
Member

Choose a reason for hiding this comment

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

I don't really understand the "As seen in the representation" part.

###############################################################################
# We fit the sum model to the empirical variogram using GSTools' built-in
# fitting capabilities. As seen in the representation, the variances and length
# scales of the individual models can be accessed by the attributes
Copy link
Member

Choose a reason for hiding this comment

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

I would also generalise this from "the variances and length scales" to "the properties of the individual models, e.g. variance and length scales" or something similar.

raise ValueError(msg)
ids = range(len(summod))
if fail := set(skip) - set(ids):
msg = f"SumModel.set_var_weights: skip ids not valid: {fail}"
Copy link
Member

Choose a reason for hiding this comment

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

"skip invalid ids"

fix this further down too, please.

var_sum = sum(summod.models[i].var for i in skip)
var_diff = var - var_sum
if var_diff < 0:
msg = "SumModel.set_var_weights: skipped variances to big."
Copy link
Member

Choose a reason for hiding this comment

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

skipped too large variances

len_sum = sum(summod[i].len_scale * summod.ratios[i] for i in skip)
len_diff = len_scale - len_sum
if len_diff < 0:
msg = "SumModel.set_len_weights: skipped length scales to big."
Copy link
Member

Choose a reason for hiding this comment

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

Please fix grammar

return para, sill, anis, sum_cfg


def _check_sill(model, para_select, sill, bnd, sum_cfg):
Copy link
Member

Choose a reason for hiding this comment

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

Please add a short description of what exactly is checked.

Comment on lines +638 to +670
if is_sum:
if sum_cfg["var_size"] > 0:
var_vals = popt[para_skip : para_skip + sum_cfg["var_size"]]
para_skip += sum_cfg["var_size"]
if sum_cfg["var_fix"]:
model.set_var_weights(
stick_breaking_uniform(var_vals),
sum_cfg["var_skip"],
sum_cfg["fix"]["var"],
)
else:
for i, val in zip(sum_cfg["var_fit"], var_vals):
setattr(model, f"var_{i}", val)
if sum_cfg["len_size"] > 0:
len_vals = popt[para_skip : para_skip + sum_cfg["len_size"]]
para_skip += sum_cfg["len_size"]
if sum_cfg["len_fix"]:
model.set_len_weights(
stick_breaking_uniform(len_vals),
sum_cfg["len_skip"],
sum_cfg["fix"]["len_scale"],
)
else:
for i, val in zip(sum_cfg["len_fit"], len_vals):
setattr(model, f"len_scale_{i}", val)
for i in range(model.size):
fit_para[f"var_{i}"] = model.vars[i]
fit_para[f"len_scale_{i}"] = model.len_scales[i]
# handle sill
if sill is not None and para["var"]:
nugget = sill - model.var
fit_para["nugget"] = nugget
model.nugget = nugget
Copy link
Member

Choose a reason for hiding this comment

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

There are quite a few pretty large if is_sum branches all over the place here. Maybe you can find a better structure for the code in this module?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Refactoring Code-Refactoring needed here
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants