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

inverse assignment of Q[:]=value should not break into individual indices #83

Open
rkishony opened this issue Oct 12, 2021 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@rkishony
Copy link
Collaborator

when inverting element-wise functions, need to preserve colon indexing. This is important because of three reasons:

  1. currently we get a long and thereby incomprehensible override_list
  2. speed
  3. we can get unexpected behaviors. see here:
N = iquib([4,3])
A = np.random.randint(0,10,N)
A.allow_overriding = True
B = A + 10

B[:,1] = 0
print(B.get_value())

N[0] = 6
print(B.get_value())

A.get_override_list()

now:

print(B.get_value())

yeilds:

[[16  0 17]
 [16  0 19]
 [13  0 11]
 [16  0 14]]

but then:

N[0] = 6
print(B.get_value())

yields

[[10  0 14]
 [18  0 10]
 [17  0 13]
 [16  0 17]
 [13 15 12]
 [13 12 12]]
@rkishony rkishony added the bug Something isn't working label Oct 12, 2021
@kmaork kmaork added enhancement New feature or request and removed bug Something isn't working labels Oct 17, 2021
@rkishony
Copy link
Collaborator Author

@kmaork:
In addition to the generic inverter, a quib should first offer to see if it can apply a quib-speciifc inverter.
for example, np.transpose will check: if it was provided indices in at least the first two positions, it will replace the order of these positions. otherwise (if there is anything non-ordinary) - it will defer to the generic inverter

as discussed: will appreciate if you can take on yourself the implementation of one such case-specific inverter, say for np.transpose, or element-wise, or both

@rkishony
Copy link
Collaborator Author

perfect.
one comment though:
I think better that the test
if not (isinstance(indices, tuple) and len(indices) == self.get_ndim()), currently done generally in _tailored_forward_translate_indices, will instead be done by each of the function-specific translators themselves.
This way, some (future / more advanced) translators will be able to take care of additional cases if they can.

So my suggestion:
The function-specific translator gets the "right-of-first-refusal": it is called first and can either translate or return 'i cannot translate' (None), in which case the generic translator will operate.

@rkishony
Copy link
Collaborator Author

As of function-specific inverse assignment:
it should be able to change not only the indices but also the value.

consider this case as an example:

A = iquib(x2d) # where x2d is a 2d array
B = np.transpose(A)
B.assign_value[y2d] # where y2d is a new 2d array

the transpose-specific inverse assigned should transpose the value (here, y2d), rather than meddling with the path (which in this case is just []).

So to conclude:
the assignment inverter can decide to change the value, the path or both. Or to say "I cannot invert" and this will go back to the generic inverter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants