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

check_inputs is too strict with multivariate coordinates #1833

Open
2 tasks done
coxipi opened this issue Jul 8, 2024 · 0 comments
Open
2 tasks done

check_inputs is too strict with multivariate coordinates #1833

coxipi opened this issue Jul 8, 2024 · 0 comments
Assignees
Labels
bug Something isn't working sdba Issues concerning the sdba submodule. standards / conventions Suggestions on ways forward
Milestone

Comments

@coxipi
Copy link
Contributor

coxipi commented Jul 8, 2024

Setup Information

  • Xclim version: '0.50.1-dev.1'
  • Python version:
  • Operating System:

Description

I think I should be able to use stacked periods & variables together:

ref, sim  = [stack_periods(ds, window=30) for ds in [ref, sim]]
daref,dasim  = [sdba.stack_variable(ds, window=30) for ds in [ref, sim]]
sdba.EmpiricalQuantileMapping.train(ref=daref.isel(period=0), hist=dasim.isel(period=0)).adjust(sim=dasim.isel(period=1))

Steps To Reproduce

from xclim.core.calendar import stack_periods
from xclim import sdba
from xclim.testing import open_dataset
ds = open_dataset("sdba/ahccd_1950-2013.nc")

# success
ds = stack_periods(ds, window=30)
sdba.BaseAdjustment._check_inputs(*[ds.isel(period=0), ds.isel(period=1)], group=sdba.Grouper("time"))

da = sdba.stack_variables(ds)
# fail
sdba.BaseAdjustment._check_inputs(*[da.isel(period=0), da.isel(period=1)], group=sdba.Grouper("time"))

This comes from this very strict comparison on multivariate coordinates mvcrds:

if mvcrds and (
    not all(mvcrds[0].equals(mv) for mv in mvcrds[1:])
            or len(mvcrds) != len(inputs)
        ):
            coords = {mv.name   for mv in mvcrds}
            raise ValueError(
                f"Inputs have different multivariate coordinates: {', '.join(coords)}."
            )

Additional context

I guess we want to be very strict with a multivariate coordinate because it's a weird object? period is a special type of coordinate where we accept in the context of bias adjustment, it can differ on various datasets

I could skip_input_checks or drop the period coordinate, it's just a bit impractical

Contribution

  • I would be willing/able to open a Pull Request to address this bug.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@coxipi coxipi added the bug Something isn't working label Jul 8, 2024
@Zeitsperre Zeitsperre added standards / conventions Suggestions on ways forward sdba Issues concerning the sdba submodule. labels Dec 10, 2024
@Zeitsperre Zeitsperre added this to the v0.55.0 milestone Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sdba Issues concerning the sdba submodule. standards / conventions Suggestions on ways forward
Projects
None yet
Development

No branches or pull requests

2 participants