-
Notifications
You must be signed in to change notification settings - Fork 81
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
Add minimal CQM implementation #839
Conversation
Still need to fix the windows bug obviously. This also will need a pretty serious second pass when we add |
3b0a614
to
d15798e
Compare
Codecov Report
@@ Coverage Diff @@
## main #839 +/- ##
==========================================
+ Coverage 88.26% 88.52% +0.25%
==========================================
Files 70 71 +1
Lines 5862 6046 +184
==========================================
+ Hits 5174 5352 +178
- Misses 688 694 +6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from a few minor issues, LGTM!
header_data = json.dumps(data, sort_keys=True).encode('ascii') | ||
header_data += b'\n' | ||
header_data += b' '*(64 - (len(prefix) + 6 + len(header_data)) % 64) | ||
header_len = np.dtype('<u4').type(len(header_data)).tobytes() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did just a quick check, but it looks .tobytes()
in this setup actually always uses native byteorder. I believe your intention was to use little endian.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, excellent catch
In [6]: np.frombuffer(np.dtype('<u4').type(256).tobytes(), '<u4')
Out[6]: array([256], dtype=uint32)
In [7]: np.frombuffer(np.dtype('>u4').type(256).tobytes(), '<u4')
Out[7]: array([256], dtype=uint32)
In [8]: np.frombuffer(np.dtype('>u4').type(256).tobytes(), '>u4')
Out[8]: array([65536], dtype=uint32)
In [9]: np.frombuffer(np.dtype('<u4').type(256).tobytes(), '>u4')
Out[9]: array([65536], dtype=uint32)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we have the same bug in BQM and DQM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. Investigating
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a fix I'll make in a followup PR.
New Features ------------ * Add `add_bqm` method to C++ BinaryQuadraticModel #821, #823 * Add `Structured.valid_bqm_graph` method for verifying input problem structure #832 * Reimplement `BinaryQuadraticModel` to use new C++ code #828 * `BinaryQuadraticModel` can now be manipulated symbolically #834 * `load` function can now load all model types #841, #843 * `DiscreteQuadraticModel` now has an `.offset` attribute #838 * Add `ConstrainedQuadraticModel` class #839 Fix --- * Fix type promotions in binary quadratic models with object biases #836
Allows the construction of binary constrained quadratic models.