Refactor dwave.optimization.generators #42
Conversation
arcondello
commented
Jul 3, 2024
arcondello
commented
- Clean up pep8 errors and whitespace
- Remove redundant tests in tests/test_examples.py
- Refactor input argument checking/coercion
- Slightly generalize some of the generators
| raise ValueError(f"`{argname}` must be a square 2D array-like of non-negative numbers") | ||
| # we do "size up" | ||
| while array.ndim < ndim: | ||
| array = array[np.newaxis] |
There was a problem hiding this comment.
I'm not quite sure what is intended with this clever trick but I doubt it is this:
model = job_shop_scheduling(times=[1, 2, 3, 4], machines=[4, 3, 2, 1])
ValueError: arrays must have the same shape
I think the code will be clearer if, for a requirement on square matrices, you check that array.shape[0] == array.shape[1] and don't upsize.
There was a problem hiding this comment.
That error was present in the original code. Turns out it fails on main for every non-square times/machines. Will fix.
Re the dimension increase, I am following the pattern of np.atleast_2d(), which is typical in this sort of place.
There was a problem hiding this comment.
model = job_shop_scheduling(times=[[1], [2], [2], [1]], machines=[[1], [1], [1], [1]])
ValueError: cannot do a reduction on an empty array with an operation that has no identity without supplying an initial value
This is probably fine in this PR, these parameters probably should be accepted, so might need a separate PR to update the model.
There was a problem hiding this comment.
Ah, that's a different bug. Will fix that one too.
arcondello
force-pushed
the
generators-refactor
branch
from
7b22af9 to
6e9ee33
Compare
| raise ValueError(f"`{argname}` must be a square 2D array-like of non-negative numbers") | ||
| # we do "size up" | ||
| while array.ndim < ndim: | ||
| array = array[np.newaxis] |
There was a problem hiding this comment.
model = job_shop_scheduling(times=[[1], [2], [2], [1]], machines=[[1], [1], [1], [1]])
ValueError: cannot do a reduction on an empty array with an operation that has no identity without supplying an initial value
This is probably fine in this PR, these parameters probably should be accepted, so might need a separate PR to update the model.
dwave/optimization/generators.py
Outdated
| array = np.atleast_2d(np.asarray(array)) | ||
| except (ValueError, TypeError): | ||
| raise ValueError(f"`{argname}` must be a square 2D array-like of non-negative numbers") | ||
| # we do "size up" |
There was a problem hiding this comment.
Can you add a comment here to give the why?
* Clean up pep8 errors and whitespace * Remove redundant tests in tests/test_examples.py * Refactor input argument checking/coercion * Slightly generalize some of the generators
arcondello
force-pushed
the
generators-refactor
branch
from
e290128 to
3f29e7b
Compare
