Skip to content

gh-1021: Add decorator for np conversions#1137

Draft
prady0t wants to merge 3 commits into
glass-dev:mainfrom
prady0t:add-decorator-for-np-conversions
Draft

gh-1021: Add decorator for np conversions#1137
prady0t wants to merge 3 commits into
glass-dev:mainfrom
prady0t:add-decorator-for-np-conversions

Conversation

@prady0t

@prady0t prady0t commented Jun 16, 2026

Copy link
Copy Markdown

Description

Addresses #1021

Checks

  • Is your code passing linting?
  • Is your code passing tests?
  • Have you added additional tests (if required)?
  • Have you modified/extended the documentation (if required)?
  • Have you added a one-liner changelog entry above (if required)?

prady0t and others added 3 commits June 16, 2026 17:27
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
@prady0t prady0t changed the title Add decorator for np conversions ENH: Add decorator for np conversions Jun 16, 2026
@prady0t prady0t changed the title ENH: Add decorator for np conversions gh-1021: Add decorator for np conversions Jun 16, 2026
Comment thread glass/_array_api_utils.py
dxp = default_xp(xp.__name__)
return tuple(xp.asarray(arr) for arr in dxp.tril_indices(n, k=k, m=m))

def numpy_fallback(func):

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucascolley This could be more generalised for the standard. Currently, I feel the limitation is the inability to specify parameters such as dtype for xp.asarray.

@ntessore @paddyroddy Would love some feedback.

@lucascolley lucascolley Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @betatim, I wonder whether there is any overlap between this and scikit-learn/scikit-learn#34324 which would be nice to upstream into xpx.numpy.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty much out of my purview, and @paddyroddy is on leave currently, so it might be a bit before he can answer.

In my opinion, which as I said doesn't carry much weight here, I don't think this is something where GLASS should have to innovate. We're in the cosmological simulations business, not the array business 🙂

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't a lot of context in the PR description, what I figure from a quick browse: this PR adds a decorator that converts all "arrays" that are passed to a function to numpy arrays, then calls the wrapped function and converts any "arrays" in the output back to the original namespace.

I'm not sure how much the work in scikit-learn/scikit-learn#34324 is useful for others. It feels somewhat scikit-learn specific. The tricky (imho) part is deciding when to convert to numpy, which I guess is a library specific thing? At least in scikit-learn the decision to convert to numpy is not a simple "this function is hard to support". It depends on the global config, the specific constructor arguments of the class and what the type of the input array is. Once you know you want to convert to numpy it feels like you are "home free".

For this library it looks like most of the code of the decorator deals with finding arrays in the input arguments and such infrastructure. One thing I'm wondering is what to do if the inputs use different array namespaces or if one of the inputs is a torch array and another a list. But maybe these are things aren't an issue here? In scikit-learn we allow a mix of things :-/

Conclusion, I wonder if we can make a generic fallback decorator for xpx that doesn't get mega complex and at the same time supports most of the different behaviours users would want. If the answer is "yes we can" then I think it would be a useful tool (even if I don't think we'd use it in scikit-learn).

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 this pull request may close these issues.

4 participants