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

Change dims keyword of sum functions to dim #245

Closed
leuchtum opened this issue Mar 6, 2024 · 1 comment · Fixed by #246
Closed

Change dims keyword of sum functions to dim #245

leuchtum opened this issue Mar 6, 2024 · 1 comment · Fixed by #246

Comments

@leuchtum
Copy link
Contributor

leuchtum commented Mar 6, 2024

I have come across a case where linopy does not adhere to the naming of xarray and this problem can only be solved with an unnecessary type check. Here is a short linear equation that should work with both pure xarray instances and a xarray/linopy mix.

For this let $$func(A,x,\tilde x) = A^T(x-\tilde x) = A^Tx - A^T\tilde x = u$$

where

  • $A \in \mathbb{R}^n \times \mathbb{R}^m$ is some (xarray) data matrix
  • $\tilde x \in \mathbb{R}^n$ is a (xarray) data array
  • $x \in \mathbb{R}^n$ is EITHER a (linopy) variable OR a (xarray) data array
  • $u \in \mathbb{R}^m$ is the result.

The following minimal example shows that a type check must be made in the implementation of $func$. The reason for this is only the different naming of dim and dims:

import linopy
import xarray as xr


def func(A, x, x_tilde):
    tmp = A * x - A * x_tilde
    if type(tmp) == xr.DataArray:
        return tmp.sum(dim="X")
    return tmp.sum(dims="X")


dimX = xr.DataArray(["x1", "x2", "x3"], dims=["X"])
dimU = xr.DataArray(["u1", "u2", "u3", "u4"], dims=["U"])
A = xr.DataArray(
    [[1, 1, 1, 1], [2, 2, 2, 2], [3, 3, 3, 3]],
    coords={"X": dimX, "U": dimU},
)
x_tilde = xr.DataArray([10, 20, 30], dims=["X"])

x_as_variable = linopy.Model().add_variables(coords=[dimX])
x_as_value = xr.DataArray([1, 2, 3], coords=[dimX])

print(func(A, x_as_variable, x_tilde))
print("===")
print(func(A, x_as_value, x_tilde))

with the expected output

LinearExpression (U: 4):
------------------------
[u1]: +1 var0[x1] + 2 var0[x2] + 3 var0[x3] - 140
[u2]: +1 var0[x1] + 2 var0[x2] + 3 var0[x3] - 140
[u3]: +1 var0[x1] + 2 var0[x2] + 3 var0[x3] - 140
[u4]: +1 var0[x1] + 2 var0[x2] + 3 var0[x3] - 140
===
<xarray.DataArray (U: 4)>
array([-126, -126, -126, -126])
Coordinates:
  * U        (U) <U2 'u1' 'u2' 'u3' 'u4'

In my opinion the dimskeyword should be renamed to dim. Of course this is a breaking change, but with a DeprecationWarning in the long and an alias in the short run this should be no problem.

I'm happy to submit a PR if this is something worth doing.

@FabianHofmann
Copy link
Collaborator

@leuchtum thank you for your issue. You are totally right, I and cannot remember why I did not named the keyword consistently with xarray... :(
I would be very happy about a PR!

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.

2 participants