Skip to content
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

Parse electron function strings at Electron construction #1926

Merged
merged 3 commits into from
Feb 28, 2024
Merged

Conversation

cjao
Copy link
Contributor

@cjao cjao commented Feb 6, 2024

Electron function strings were previously parsed during build_graph(). This doesn't work for electrons in sublattices since they will be built in an executor which typically won't have access to the user's source tree.

With this change, all function strings will be parsed client-side when the electron decorator is evaluated.

Tested manually using both Dask and a modified AWSBatch executor image.

  • I have added the tests to cover my changes.
  • I have updated the documentation and CHANGELOG accordingly.
  • I have read the CONTRIBUTING document.

cjao added 2 commits February 5, 2024 08:36
Waiting until `build_graph()` won't work if the graph is being built
in a remote executor without the original source code.
Copy link

codecov bot commented Feb 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (170f4d6) 84.57% compared to head (d80bbf0) 85.15%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1926      +/-   ##
===========================================
+ Coverage    84.57%   85.15%   +0.57%     
===========================================
  Files          295      181     -114     
  Lines        14502    11288    -3214     
  Branches       195        0     -195     
===========================================
- Hits         12265     9612    -2653     
+ Misses        2103     1676     -427     
+ Partials       134        0     -134     
Flag Coverage Δ *Carryforward flag
Dispatcher 92.53% <ø> (ø) Carriedforward from 170f4d6
Functional_Tests ?
SDK 79.59% <100.00%> (-0.01%) ⬇️
UI_Backend ?
UI_Frontend ?

*This pull request uses carry forward flags. Click here to find out more.

@cjao cjao marked this pull request as ready for review February 6, 2024 23:05
@cjao cjao requested a review from a team as a code owner February 6, 2024 23:05
Copy link
Member

@santoshkumarradha santoshkumarradha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cjao non blocking, but might be better to have sublattice string test as well along with this?

@cjao cjao merged commit e6ad647 into develop Feb 28, 2024
14 checks passed
@cjao cjao deleted the i-1149-fix-fstr branch February 28, 2024 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants