Skip to content

Delegating quacc @flow to jobflow @flow#3016

Merged
Andrew-S-Rosen merged 29 commits intoQuantum-Accelerators:mainfrom
vineetbansal:vb/flow_decorator
Feb 3, 2026
Merged

Delegating quacc @flow to jobflow @flow#3016
Andrew-S-Rosen merged 29 commits intoQuantum-Accelerators:mainfrom
vineetbansal:vb/flow_decorator

Conversation

@vineetbansal
Copy link
Collaborator

@vineetbansal vineetbansal commented Nov 14, 2025

Summary of Changes

>> Provide context and a description of your changes here. Make sure to reference any associated issues. <<

Requirements

Note: If you are an external contributor, you will see a comment from @buildbot-princeton. This is solely for the maintainers.

@vineetbansal
Copy link
Collaborator Author

vineetbansal commented Nov 14, 2025

TODOs and points to discuss:

  • The tests in tests/jobflow are not yet comprehensive. Ideally, all 4 combos of JOBFLOW_AUTO_SUBMIT/JOBFLOW_RESOLVE_FLOW_RESULTS would need to get tested using with change_settings.

  • This is building off a custom branch vb/flow_decorator_quacc in the fork for jobflow that differs from the one (vb/flow_decorator) in the PR in one crucial way - in the official PR the @flow decorator insists that a Job/Flow or OutputReference be returned, to keep it in line with how a Flow operates (i.e. something like flow = Flow([job1, job2], output=[job1.output, job2.output]) would have no effect on the output of the Flow, and the returned dict is still a dict of results from all the jobs in the jobs parameter). See the difference between the 2 here.

This off-PR branch exists only because tests in tests/jobflow/test_syntax.py seems to expect this to work:

  @subflow
  def add_distributed(vals, c):
      return [add(val, c) for val in vals]

This seems to be the way we use flows anyway:

    for slab in slabs:
      result = relax_job(slab)

      if static_job is not None:
          result = static_job(result["atoms"])

      results.append(result)

So perhaps we push these changes upstream or pull them downstream in quacc?

  • Another test in test_syntax.py seems to expect this to work:
    @flow
    def workflow(a, b, c):
        return mult(add(a, b), c)
    ...
    assert isinstance(workflow(1, 2, 3), jf.Flow)

So combining a Job output and a primitive in an operation. This works in Prefect but will not work in our Jobflow flow decorator without sophisticated handling. See comment by gpeteretto - "..I would not see any reasonable way of implementing this, especially for jobflow-remote or fireworks executions. ". So before we tackle this, I wanted to make sure that this is not just a test case but an actual usage pattern in quacc.

  • Do we even need a JOBFLOW_AUTO_SUBMIT? I see that in tests/jobflow/test_tutorials.py we have:
    responses = jf.run_locally(flow, ensure_success=True)
   assert responses[job2.uuid][1].output == 9

So I need to confirm if job submission is the eventual responsibility of user code. If we do support it, then most certainly I'm doing it wrong here:

        if settings.JOBFLOW_AUTO_SUBMIT:
            return run_locally(bound_jobflow_flow, ensure_success=True)

and I need to use jfremote to do this (once corresponding changes are incorporated in jfremote).

  • Start adding documentation!

@vineetbansal
Copy link
Collaborator Author

@Andrew-S-Rosen - I'm getting the same error as in the CI locally:

ModuleNotFoundError: No module named 'matgl.apps._pes_pyg'

I can try to fix this, unless you have some ideas already..

@Andrew-S-Rosen
Copy link
Member

The issues are actually slightly different I think. The issue you are getting seems to be related to the fact that the matgl dependency was updated yesterday with breaking changes, and dependabot had not yet bumped the version for me to flag and correct.

The issue in CI right now which is using the prior version of matgl is complaining about some model caching issue, probably also related to changes to the ML models in matgl that were updated on Hugging Face recently. Neither are related to quacc directly. We can chat soon about it.

Removed caching for pip packages in workflow.
@codecov
Copy link

codecov bot commented Nov 14, 2025

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.61%. Comparing base (76fb8d0) to head (8eaf782).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/quacc/wflow_tools/customizers.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3016      +/-   ##
==========================================
- Coverage   97.69%   97.61%   -0.09%     
==========================================
  Files          97       97              
  Lines        4172     4187      +15     
==========================================
+ Hits         4076     4087      +11     
- Misses         96      100       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vineetbansal vineetbansal changed the title Testing against custom fork of jobflow that supports a flow decorator Deleting quacc @flow to jobflow @flow Feb 2, 2026
@vineetbansal vineetbansal changed the title Deleting quacc @flow to jobflow @flow Delegating quacc @flow to jobflow @flow Feb 2, 2026
@Andrew-S-Rosen
Copy link
Member

@vineetbansal what is the status of this PR? Are you waiting on me with this?

@vineetbansal
Copy link
Collaborator Author

@Andrew-S-Rosen - yes, this is ready from my end and can be merged, pending any suggestions from your end.

@Andrew-S-Rosen
Copy link
Member

Works for me! Let's do this!

1 similar comment
@Andrew-S-Rosen
Copy link
Member

Works for me! Let's do this!

@Andrew-S-Rosen Andrew-S-Rosen merged commit aa0c8b3 into Quantum-Accelerators:main Feb 3, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants