Skip to content

Conversation

@danra
Copy link
Contributor

@danra danra commented Jul 11, 2025

Avoid creating two different wrappers for the same object in wrap_namespace.

Generally cleaner, but more importantly towards better ufuncs support, which will require mapping from ufuncs to their wrapped version.

Another minor pro is that this theoretically avoids needing to define grads for two names of the same object, e.g., we could define the VJP for anp.conj but not for anp.conjugate since they are actually the same wrapped object. However, it's minor and theoretical because whether or not two "equivalent" numpy functions/ufuncs are the same object is apparently an implementation detail, e.g., np.amax is documented as an alias of np.max, but they actually aren't the same object, and the equivalent ufuncs np.degrees and np.rad2deg are two different ones (tested on numpy 2.3.1). Because this might change between different numpy version, it's more robust to keep the grad definitions for both names.

Avoid creating two different wrappers for the same object in wrap_namespace.

Generally cleaner, but more importantly towards better ufuncs support,
which will require mapping from ufuncs to their wrapped version.

Another minor pro is that this theoretically avoids needing to define grads
for two names of the same object, e.g., we could define the VJP for anp.conj
but not for anp.conjugate since they are actually the same wrapped object.
However, it's minor and theoretical because whether or not two "equivalent"
numpy functions/ufuncs are the same object is apparently an implementation
detail, e.g., np.amax is documented as an alias of np.max, but they actually
aren't the same object, and the equivalent ufuncs np.degrees and np.rad2deg
are two different ones (tested on numpy 2.3.1). Because this might change
between different numpy version, it's more robust to keep the grad
definitions for both names.
Copy link
Collaborator

@agriyakhetarpal agriyakhetarpal left a comment

Choose a reason for hiding this comment

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

Thanks, @danra! This makes sense to me.

@agriyakhetarpal agriyakhetarpal requested review from Copilot and fjosw and removed request for Copilot October 19, 2025 16:14
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.

2 participants