Conversation
Summary of ChangesHello @AymenFJA, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request represents a significant architectural shift for AsyncFlow, transitioning it from a system with internal execution capabilities to a dedicated workflow orchestration layer. By removing its own execution backends and integrating with RHAPSODY, AsyncFlow can now leverage RHAPSODY's specialized AI-HPC execution features, allowing AsyncFlow to focus on managing complex dependencies and workflow logic more effectively. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request undertakes a significant migration to offload execution capabilities to RHAPSODY, which is reflected by the removal of the internal execution backend implementations. The changes in the test files correctly adapt to this by using the ConcurrentExecutionBackend from rhapsody.
However, the refactoring appears to be incomplete in src/radical/asyncflow/workflow_manager.py. While the import for BaseExecutionBackend has been removed, it is still referenced as a type hint in the create and _setup_execution_backend methods. This will cause a NameError at runtime and is a critical issue that needs to be addressed. These methods need to be updated to remove the dependency on the now-deleted BaseExecutionBackend.
Additionally, I've left a specific comment regarding a removed type hint in WorkflowEngine.__init__ that should be restored for code clarity.
| def __init__( | ||
| self, | ||
| backend: BaseExecutionBackend, | ||
| backend, |
There was a problem hiding this comment.
The type hint for the backend parameter has been removed. While the old BaseExecutionBackend is no longer applicable, it's good practice to maintain type hints for clarity and static analysis. Please add a type hint for the backend. If there isn't a specific base class from RHAPSODY to use, typing.Any would be a suitable replacement for now.
| backend, | |
| backend: Any, |
into rhapsody. Asyncflow now ships only NoopExecutionBackend and LocalExecutionBackend in a single backends.py module. External backends (e.g. from rhapsody) plug in via simple import. Also fixes missing cloudpickle import, stale type annotations, and updates all tests to use asyncflow's LocalExecutionBackend.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
This PR removes the execution backends from AsyncFlow and relies on RHAPSODY for AI-HPC execution.
AsyncFlow will no longer have execution capabilities; instead, it will be a higher entity that controls RHAPSODY and orchestrates a large scale of async workflows with complex dependencies while offloading the task execution to RHAPSODY.
From a migration perspective, the application-level frameworks such as ROSE, IMPRESS, DDSim, and FACTs will require at least 1 line of code change.