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

bqm.resize should not return a value #1091

Closed
JoelPasvolsky opened this issue Jan 31, 2022 · 5 comments · Fixed by #1093
Closed

bqm.resize should not return a value #1091

JoelPasvolsky opened this issue Jan 31, 2022 · 5 comments · Fixed by #1093
Labels
enhancement New feature or request

Comments

@JoelPasvolsky
Copy link
Contributor

Description
(

return self.data.resize
) :

return self.data.resize

returns a zero when this method is run. Why?

Steps To Reproduce

bqm.resize(3)
0

Expected Behavior
Quietly do its work.

Environment

  • OS: WIN10
  • Python version: 3.6

Additional Context
I'm in that file now, can I delete the return ?

@arcondello
Copy link
Member

arcondello commented Jan 31, 2022

We need to return a value in order to be able to raise an exception. See error return values.

We could obfuscate it, but that would result in a performance regression since this function is called frequently.

@JoelPasvolsky
Copy link
Contributor Author

What about returning the difference in number of variables between current and previous BQM rather than 0?

@arcondello
Copy link
Member

Also, to be clear,

return self.data.resize
does not return the value of zero, it is returning the method to be used. See
def forwarding_method(func):

The value is returned by

cpdef Py_ssize_t resize(self, Py_ssize_t n) except -1:
which is a cpdef function which is why we need that return value.

@arcondello
Copy link
Member

What about returning the difference in number of variables between current and previous BQM rather than 0?

Could just be the size of the resulting BQM (since -1 is what indicates an error). It's a bit redundant since that's the value being passed in but at least would be a bit more meaningful 🤷

@arcondello
Copy link
Member

What about returning the difference in number of variables between current and previous BQM rather than 0?

Changed my mind, your idea is better. We can just error on 0 instead since that should be an uncommon case if we're returning the delta.

@arcondello arcondello added the enhancement New feature or request label Jan 31, 2022
arcondello added a commit to arcondello/dimod that referenced this issue Jan 31, 2022
arcondello added a commit to arcondello/dimod that referenced this issue Jan 31, 2022
arcondello added a commit to arcondello/dimod that referenced this issue Jan 31, 2022
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

Successfully merging a pull request may close this issue.

2 participants