-
Notifications
You must be signed in to change notification settings - Fork 4
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 constructors and mutators useful downstream #5
base: main
Are you sure you want to change the base?
Conversation
c50dfc4
to
67a1139
Compare
src/qirxacc/XaccQuantum.cc
Outdated
accelerator_ = xacc::getAccelerator(accel_name); | ||
QIREE_VALIDATE(accelerator_, << "failed to create accelerator"); | ||
accelerator_->updateConfiguration({{"shots", static_cast<int>(shots)}}); | ||
set_accelerator_and_shots(accel_name, shots); |
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.
Is there a reason to "create and then initialize" the class like this? Best practices say initialization should take place at construction. It improves performance sometimes, but more importantly it helps "reasoning" about the state of the code because you don't have to worry about some other part of the code changing the state of an object that you got.
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.
Okay, I modified it to duplicate that chunk of code
This is useful when a user generates an llvm::Module that may have multiple entry points, as the Qwerty compiler/runtime does.
In my experience, it has been easier to treat XaccQuantum as a singleton to avoid issues with XACC initializing itself twice. It is probably cleaner to pass (accelerator, shots) via qiree::Executor, but this solves my immediate implementaiton problem in the Qwerty compiler/runtime.
ff5bc0c
to
454b207
Compare
454b207
to
079e397
Compare
These are useful for me downstream in the Qwerty compiler/runtime.
It would be nicer to pass the accelerator and shots as arguments to
qiree::Executor
somehow, but this solves my immediate engineering problem.Testing
Once rebased against #4, this builds and passes
ctest
. I previously tested this code end-to-end in the Qwerty runtime but the code is in a state where it will be tough to do that again.