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

Problem with bin size in spatial_skill #157

Open
daniel-caichac-DHI opened this issue Jan 10, 2023 · 1 comment · May be fixed by #395
Open

Problem with bin size in spatial_skill #157

daniel-caichac-DHI opened this issue Jan 10, 2023 · 1 comment · May be fixed by #395
Assignees

Comments

@daniel-caichac-DHI
Copy link
Collaborator

Hi,

I think that there is some problem with bin selection in the spatial plots when the bins are not fully defined by the user
as (min,max,delta).
To exemplify this. I got track observations between Lat=[+31.4 , +65.62] deg
image

then if I define my bin spacing as 0.25deg and make a spatial skill starting from +32deg to +66deg I get data as expected between 32 and 66deg
image
image

but if instead I just go with binsize=0.25 and let fmskill define the min and max Latitude I get
image
image
ie, I see that my data now starts a roughly Latitude=+36deg even though the bin size is still 0.25deg.
I am somehow losing 4deg of data (~400 km of satellite data).
Not ideal. Most likely a bug.

cc: @Hendrik1987

@Hendrik1987
Copy link
Contributor

I can confirm this after discussion with @daniel-caichac-DHI . I do further remember that the 'auto binning' when the user provide binsize was a challenge when implementing the spatial skill in the first place. Then I looked mainly at a global data set and likely have not tested all special cases sufficiently.
We need to review after else: https://github.com/DHI/fmskill/blob/e17d88db6bb473e48ddede4c6d7dbd267cce04f9/fmskill/comparison.py#L681
Quick fix is to not allow binsize argument.

@ecomodeller ecomodeller self-assigned this Jan 17, 2024
@ecomodeller ecomodeller linked a pull request Jan 17, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants