Skip to content

Enable electrochem auto-selection for charged reactive species - #3009

Merged
JacksonBurns merged 1 commit into
mainfrom
copilot/add-electrochemistry-trigger
Sep 14, 2026
Merged

JacksonBurns merged 1 commit into
mainfrom
copilot/add-electrochemistry-trigger

Conversation

Copilot AI commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

auto library selection currently enables electrochemistry only via ELECTROCHEM_ELEMENTS (currently Li), so charged CO2RR inputs can miss electrochem libraries/families. This change broadens detection to include ionic chemistry while preserving existing element-based behavior.

  • Detection logic (rmgpy/data/auto_database.py)

    • In detect_chemistry(), mark has_electrochem when any reactive initial species has non-zero net charge.
    • Preserve current Li-trigger path and combine both signals so charge-based detection is not overwritten later.
  • Behavioral invariant

    • has_electrochem is now true if either condition is true:
      1. at least one reactive species is charged, or
      2. an electrochem trigger element is present (ELECTROCHEM_ELEMENTS).
  • Targeted coverage (test/rmgpy/data/autoDatabaseTest.py)

    • Added case for charged-species trigger ([H+]).
    • Added explicit case retaining legacy element trigger ([Li]).
# detect_chemistry() core change
if mol.get_net_charge() != 0:
    profile.has_electrochem = True

profile.has_electrochem = (
    profile.has_electrochem
    or bool(profile.elements_present & ELECTROCHEM_ELEMENTS)
)

Copilot AI changed the title Improve auto electrochem detection for charged species Enable electrochem auto-selection for charged reactive species Sep 14, 2026
Copilot AI requested a review from rwest September 14, 2026 15:45

@rwest rwest left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, but it's doing exactly what I told it to do in the prompt, so I'm not an independent reviewer. Someone else should approve.

@rwest
rwest marked this pull request as ready for review September 14, 2026 15:50

@JacksonBurns JacksonBurns 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.

Makes sense to me!

Co-authored-by: rwest <93807+rwest@users.noreply.github.com>
@JacksonBurns
JacksonBurns force-pushed the copilot/add-electrochemistry-trigger branch from 2cf315a to e724a8d Compare September 14, 2026 20:02
@JacksonBurns
JacksonBurns merged commit cef1873 into main Sep 14, 2026
17 checks passed
@JacksonBurns
JacksonBurns deleted the copilot/add-electrochemistry-trigger branch September 14, 2026 21:49
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.

3 participants