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

Call processed variables with custom spatial coordinates #3441

Open
rtimms opened this issue Oct 11, 2023 · 5 comments
Open

Call processed variables with custom spatial coordinates #3441

rtimms opened this issue Oct 11, 2023 · 5 comments
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve

Comments

@rtimms
Copy link
Contributor

rtimms commented Oct 11, 2023

After #3234 ProcessedVariable objects now automatically take the names of the domains to plot automatically from spatial variables, but __call__ still expects the spatial variables to be x, y, z, r, or R, see:

    def __call__(self, t=None, x=None, r=None, y=None, z=None, R=None, warn=True):
        """
        Evaluate the variable at arbitrary *dimensional* t (and x, r, y, z and/or R),
        using interpolation
        """
        kwargs = {"t": t, "x": x, "r": r, "y": y, "z": z, "R": R}
        # Remove any None arguments
        kwargs = {key: value for key, value in kwargs.items() if value is not None}
        # Use xarray interpolation, return numpy array
        return self._xr_data_array.interp(**kwargs).values

We should make it possible to pass extra kwargs to __call__.

As a workaround, you can do something like

# variable that depends on a  spatial variable named "s" and time "t"
my_var_data_array = sim.solution["My var"]._xr_data_array

def my_var_eval(s, t):
    return my_var_data_array.interp(s=s, t=t).values
@rtimms rtimms added difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve labels Oct 11, 2023
@brosaplanella
Copy link
Member

Yes! I thought of this when doing #3234 but didn't open an issue for it.

@Saswatsusmoy
Copy link

Can we do something like this?

-Allow passing arbitrary keyword arguments to call by adding **kwargs
-Get list of expected args from the method signature
-Get args passed to call and kwargs
-Construct coord dict from expected args that were passed
-Update with any extra kwargs
-Pass coord dict to interp

@rtimms
Copy link
Contributor Author

rtimms commented Oct 24, 2023

@Saswatsusmoy sounds good, would you like to give it a go?

@Saswatsusmoy
Copy link

@rtimms I have tried and modified the call method to accept **kwargs as a parameter. This will allow any additional keyword arguments to be passed to the method.

Saswatsusmoy added a commit to Saswatsusmoy/PyBaMM that referenced this issue Jan 29, 2024
…Call-processed-variables-with-custom-spatial-coordinates
Saswatsusmoy added a commit to Saswatsusmoy/PyBaMM that referenced this issue Jan 30, 2024
…Call-processed-variables-with-custom-spatial-coordinates
arjxn-py added a commit to Saswatsusmoy/PyBaMM that referenced this issue Feb 7, 2024
…sed-variables-with-custom-spatial-coordinates
@rtimms
Copy link
Contributor Author

rtimms commented Nov 22, 2024

@valentinsulzer id this something that can be fixed as part of #4600 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants