Skip to content

Only process - #653

Open
alongd wants to merge 3 commits into
mainfrom
only_process
Open

alongd wants to merge 3 commits into
mainfrom
only_process

Conversation

@alongd

@alongd alongd commented May 10, 2023 •

Copy link
Copy Markdown
Member

We'd like ARC to immediately process thermo/kinetics after all relevant jobs have ended.
This PR takes us one small step closer to this goal, adding an only_process arg to the input (restart) file, to request ARC to only process what it can and not spawn new jobs in this restart mode when only_process is True.

Also, save the species adjlist if given. Some species are not read correctly in RMG (especially hyperventilate N and S containing species), which results in excluding them from the final thermo library. Here we use the user-provided adjlist (which is the best way to define these species) if given.

Comment thread arc/plotter.py Fixed
Comment thread functional/functional_test.py Fixed
Comment thread functional/functional_test.py Fixed
Comment thread arc/species/zmat.py Fixed
Comment thread arc/species/zmat.py Fixed
Comment thread arc/job/adapters/ts/kinbot_test.py Fixed
Comment thread arc/common.py Fixed
@codecov

codecov Bot commented May 14, 2023 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.05%. Comparing base (d9f47ab) to head (991b3bc).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #653      +/-   ##
==========================================
- Coverage   67.12%   67.05%   -0.08%     
==========================================
  Files         123      123              
  Lines       43602    43608       +6     
  Branches    11144    11146       +2     
==========================================
- Hits        29268    29240      -28     
- Misses      11197    11224      +27     
- Partials     3137     3144       +7     
Flag Coverage Δ
functionaltests 67.05% <ø> (-0.08%) ⬇️
unittests 67.05% <ø> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces an "only process" mode to ARC, allowing users to skip job spawning and only process existing calculations when restarting. The implementation adds an only_process parameter to control whether new jobs should be spawned, and improves species handling by preserving user-provided adjacency lists for better accuracy with complex nitrogen and sulfur species.

  • Adds only_process parameter to ARC and Scheduler classes to control job spawning behavior
  • Preserves user-provided adjacency lists in species definitions for better molecular representation
  • Updates test cases to include arkane_file attribute in expected output dictionaries

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
arc/scheduler.py Adds only_process parameter to Scheduler class and prevents job spawning when enabled
arc/main.py Adds only_process parameter to ARC class and passes it to Scheduler
arc/reaction/reaction_test.py Updates test expectations to include arkane_file attribute in species dictionaries

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread arc/scheduler.py
Comment on lines +434 to +436
if only_process:
pass
elif species.is_monoatomic():

Copilot AI Aug 25, 2025

Copy link

Choose a reason for hiding this comment

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

The if only_process: pass block is redundant and can be simplified by combining the conditions. Consider refactoring to elif not only_process and species.is_monoatomic():

Suggested change
if only_process:
pass
elif species.is_monoatomic():
elif not only_process and species.is_monoatomic():

Copilot uses AI. Check for mistakes.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

only_process was added with no test exercising it. Covers the arc/main.py side:
stored on the instance, written to the restart dict only when truthy (so existing
restart files are unchanged), and accepted back as an input key.

The arc/scheduler.py side is deliberately not covered here: the schedule_jobs()
guard is already short-circuited by testing=True, and reaching the 'if only_process'
branch requires a Scheduler over a species that would otherwise spawn real sp/freq
jobs.
kfir4444 added a commit that referenced this pull request Oct 4, 2026
…rift (#1066)

## What

Adds a `codecov.yml` (23 lines, no code change):

- `coverage.status.project.default.threshold: 1%`
- `coverage.status.patch.default.target: 80%`

## Why

ARC has no Codecov configuration at all — `.coveragerc` configures
coverage.py's *collection*, not
Codecov's *status checks*. With no config, Codecov falls back to its
built-in defaults, under which
the `project` status is `target: auto` with a **0% threshold**: total
coverage may never dip by any
amount relative to the base commit.

The consequence is that nearly every PR adding code covered below the
repository average (~67%)
reports a failing check while every test job, CodeQL and the functional
suite are green. Observed
on the current open set, all red on `codecov/project` alone:

| PR | status |
|---|---|
| #653 | `67.03% (-0.10%)` → `67.05% (-0.08%)` after adding a genuine
unit test |
| #860, #1016, #1039, #1050, #1054, #1055, #1056, #1057 | red on
`codecov/project`, everything else green |

On #653, `codecov/patch` simultaneously reported *"Coverage not
affected"* — the gate that actually
judges whether the PR's new code is tested was already satisfied. A
check that fails on
one-tenth of a percentage point trains reviewers to ignore it, which
costs us the patch signal too.

## What this does and does not change

- **Does:** lets total coverage drift up to 1% without failing the
status.
- **Does:** keep an explicit `patch` target, so the coverage of a PR's
own new or changed lines
remains a gating signal. This is the stricter half of the config, not a
loosening.
- **Does not:** touch any test or source file, change `.coveragerc`, or
alter what coverage is
  collected.
- **Does not:** raise ARC's actual coverage. That is a separate and much
larger piece of work.

Note that Codecov re-evaluates statuses on new uploads, so already-open
PRs will pick this up on
their next CI run rather than retroactively.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants