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 'temporal', 'spatial_dim' and 'geo_scale' attributes to Covmodel #308

Merged
merged 40 commits into from
Jun 15, 2023

Conversation

MuellerSeb
Copy link
Member

@MuellerSeb MuellerSeb commented Jun 6, 2023

Closes #223, #282

This PR adds a boolean temporal and spatial_dim attribute to the CovModel class as well as geo_scale to CovModel and vario_estimate.

Setting temporal=True indicates that the model should be seen as a metric spatio-temporal model. This was possible before by increasing the model dimension by one but this feature basically enables using spatio-temporal model with latlon coordinates. spatial_dim was added so the spatial dimension could be set explicitly and not implicitly by increasing only dim by 1.

Also a geo_scale attribute was added to be able to get meaningful length-scales and bins for latlon models. Before we always used the rescale attribute to do that, but that was rather a hack. Now both can be used to set the sphere radius and to rescale the length-scale. In addition to that I added KM_SCALE, DEGREE_SCALE and RADIAN_SCALE constant that can be used as radius to get a length-scale in km, degree or radian as some people expect the length-scale to be in degree when working latlon coordinates.

Another dimension indicator attribute was added to the CovModel class:

  • spatial_dim: The truly spatial dimension of the model (2 for latlon, otherwise dim or dim-1 (if temporal=True))
    • in contrast to dim: the internal model dimension (4 for latlon=True and temporal=True for example)
    • in contrast to field_dim: this is the parametric dimension of the generated field (3 for latlon=True and temporal=True, 2 for latlon=True and temporal=False for example)

Works like this:

import numpy as np
import gstools as gs

model = gs.Matern(
    temporal=True,
    latlon=True,
    len_scale=[1000, 100],
    geo_scale=gs.KM_SCALE,
)

lat = lon = np.linspace(-80, 81, 50)
time = np.linspace(0, 777, 50)
srf = gs.SRF(model, seed=1234)
field = srf.structured((lat, lon, time))
srf.plot()

Figure_1
Figure_2

@MuellerSeb MuellerSeb added the enhancement New feature or request label Jun 6, 2023
@MuellerSeb MuellerSeb added this to the v1.5 milestone Jun 6, 2023
@MuellerSeb MuellerSeb requested a review from LSchueler June 6, 2023 13:19
@MuellerSeb MuellerSeb self-assigned this Jun 6, 2023
@MuellerSeb
Copy link
Member Author

MuellerSeb commented Jun 6, 2023

@LSchueler would it make sense to you to move the time axis position in the pos tuple to the front in case of latlon coordinates? This would then follow CF-Conventions: T-Lat-Lon (and not Lat-Lon-T) which is basically T-Y-X (fully inversed axis order in contrast to normal (x,y,z,...,t)).

For non latlon fields, I would keep (x,y,z,...,t).

This was referenced Jun 6, 2023
@MuellerSeb MuellerSeb changed the title Add time attribute to Covmodel Add time and radius attribute to Covmodel Jun 8, 2023
@MuellerSeb
Copy link
Member Author

MuellerSeb commented Jun 9, 2023

@LSchueler Maybe radius is a bit misleading, since it is a rather technical term with respect to the internal representation of the yadrenko-variogram. May we could call this attribute geo_scale witch is a bit more self-descriptive. What do you think?

@MuellerSeb MuellerSeb changed the title Add time and radius attribute to Covmodel Add 'time' and 'geo_scale' attributes to Covmodel Jun 12, 2023
@MuellerSeb
Copy link
Member Author

@LSchueler I like the geo_scale attribute. Next question, following #282 is, if we want to add geo_scale also to the vario_estimate routine so the bin_centers have a meaningful unit?

@MuellerSeb MuellerSeb changed the title Add 'time' and 'geo_scale' attributes to Covmodel Add 'temporal' and 'geo_scale' attributes to Covmodel Jun 13, 2023
@MuellerSeb
Copy link
Member Author

@LSchueler all fixes implemented. I would also vote against the CF conventions and maybe have that with #140

@MuellerSeb
Copy link
Member Author

@LSchueler Only question I still have is, if dim should really be automatically increased by 1 if temporal=True.

For example if you want a model with 3 spatial dimensions and a time dimension, which option looks better to you:

model = gs.Gaussian(dim=3, temporal=True)

or

model = gs.Gaussian(dim=4, temporal=True)

In both cases model.dim would be 4 and model.spatial_dim would be 3.

@MuellerSeb
Copy link
Member Author

MuellerSeb commented Jun 14, 2023

I added spatial_dim to solve this. dim now always really refers to the model dimension. spatial_dim could be used instead, when defining a spatio-temporal model:

model = gs.Gaussian(temporal=True, spatial_dim=3)
model.dim == 4
model.spatial_dim == 3

This is equal to:

model = gs.Gaussian(temporal=True, dim=4)

@MuellerSeb MuellerSeb changed the title Add 'temporal' and 'geo_scale' attributes to Covmodel Add 'temporal', 'spatial_dim' and 'geo_scale' attributes to Covmodel Jun 14, 2023
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.

Nice work! I think you are nearly there.

CHANGELOG.md Outdated Show resolved Hide resolved
src/gstools/variogram/variogram.py Outdated Show resolved Hide resolved
src/gstools/variogram/variogram.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated 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.

:-D Wow, my review wasn't very thorough, was it?

LGTM!

@MuellerSeb MuellerSeb merged commit 83c85f8 into main Jun 15, 2023
@MuellerSeb MuellerSeb deleted the add_time_feature branch June 15, 2023 15:58
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
2 participants