-
Notifications
You must be signed in to change notification settings - Fork 23
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
fix precomputation in get_objective_maxcut.py #11
Conversation
The tests are failing because the GPU test is not skipped properly if no GPU is available (see logs in the check above):
|
tests/test_maxcut.py
Outdated
for u, v, w in G.edges(data=True): | ||
w["weight"] = np.random.rand() | ||
precomputed_cuts = precompute_energies(maxcut_obj, N, w=get_adjacency_matrix(G)) | ||
simclass = qokit.fur.choose_simulator("gpu") |
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.
Shouldn't this function call fail if no GPU is present? I checked on a CPU-only instance, and it does not throw an error
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.
precompute_energies
is the CPU-only function, so it shouldn't fail. If you use the simulator from choose_simulator("gpu")
it should fail on initialization.
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.
Github comments are a bit ambiguous; I was referring to line 91 in particular. Running the following two lines in a CPU-only instance does not throw an error:
import qokit
qokit.fur.choose_simulator("gpu")
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.
Yes, this is expected, since choose_simulator
returns the class, which is then used in later steps of simulation. Using this class will raise an error
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.
Tests should pass both on CPU and GPU before the review is requested
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.
LGTM, thanks for fixing promptly!
fixes #9