-
Notifications
You must be signed in to change notification settings - Fork 17
Exclusive access task #1025
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
Exclusive access task #1025
Conversation
|
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.
The constructor of the class needs some work to make it not crash the CI if you have any questions lets just set up a pair coding session to fix the issue.
src/bloqade/analog/task/base.py
Outdated
@property | ||
@abc.abstractmethod | ||
def parallel_decoder(self) -> ParallelDecoder: ... | ||
|
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.
This shouldn't be deleted, this is needed to get things working with the Report object.
src/bloqade/analog/task/exclusive.py
Outdated
|
||
|
||
@ExclusiveRemoteTask.set_serializer | ||
|
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.
removing a space here.
src/bloqade/analog/task/exclusive.py
Outdated
class ExclusiveRemoteTask(CustomRemoteTaskABC): | ||
def __init__( | ||
self, | ||
task_ir: QuEraTaskSpecification, | ||
metadata: Dict[str, ParamType], | ||
parallel_decoder: ParallelDecoder | None, | ||
http_handler: HTTPHandlerABC | None = HTTPHandler(), | ||
task_id: str = None, | ||
task_result_ir: QuEraTaskResults = None, | ||
): | ||
self._http_handler = http_handler | ||
self._task_ir = task_ir | ||
self._metadata = metadata | ||
self._parallel_decoder = parallel_decoder | ||
float_sites = list( | ||
map(lambda x: (float(x[0]), float(x[1])), task_ir.lattice.sites) | ||
) | ||
self._geometry = Geometry( | ||
float_sites, task_ir.lattice.filling, parallel_decoder | ||
) | ||
self._task_id = task_id | ||
self._task_result_ir = task_result_ir |
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.
As per our discussion 1 on 1, use a dataclass to clean up this implementation, Also you should use default_factories for the default values instead of instances of objects that are mutable.
src/bloqade/analog/task/exclusive.py
Outdated
else: | ||
self._task_result_ir = QuEraTaskResults( | ||
task_status=QuEraTaskStatusCode.Failed) | ||
print(self.task_result_ir) |
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.
print(self.task_result_ir) |
removing debug print statements.
Also you should add a test using the Test interface we discussed. |
…g in HTTPHandler and ExclusiveRemoteTask
☂️ Python Coverage
Overall Coverage
New Files
Modified FilesNo covered modified files...
|
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
Updates:
Test
|
Looks good to me, my test script runs 👍. @weinbe58 merge at your convenience. |
Add exclusive access
The environment variables should be set before import ExclusiveRemoteTask.
Test
Waiting time