GeoStat-Framework / GSTools-Core

A Rust implementation of the core algorithms of GSTools.
GNU Lesser General Public License v3.0
10 stars 0 forks source link

Is the dim parameter of variogram_directional required? #10

Closed adamreichold closed 2 years ago

adamreichold commented 2 years ago

variogram_directional has an explicit parameter dim that is part of the Python API but is only even used to constrain the first dimension of the pos array and the second dimension of the direction array. Is this parameter really necessary? Why can't calling code just slice pos and direction itself using e.g. pos[:dim,:]?

adamreichold commented 2 years ago

This seems to be coming from

pos, __, dim = format_(un)struct_pos_shape(
    pos, field.shape, check_stacked_shape=True
)

in variogram.py which seem ensure the right shape any, i.e.

pos = pos.reshape((dim, -1))

in geometric.py.

Similarly, the shape of direction seems to be enforced already

if direction.shape[1] != dim:
    raise ValueError(f"Can't interpret directions: {direction}")

so that it seems that the parameter is really unnecessary.

(Thinking further ahead, having format_(un)struct_pos_shape implemented in Rust would allow to drop the whole "collect it into a Vec of ShortVec" rigmarole by reshaping the input directly into that form.)