Skip to content

Filter reactions where one of the reactants is the product#148

Merged
kmaziarz merged 3 commits into
microsoft:mainfrom
jla-gardner:jla-gardner/remove-reactant-product-repetition
Jun 25, 2026
Merged

Filter reactions where one of the reactants is the product#148
kmaziarz merged 3 commits into
microsoft:mainfrom
jla-gardner:jla-gardner/remove-reactant-product-repetition

Conversation

@jla-gardner

@jla-gardner jla-gardner commented Feb 13, 2026

Copy link
Copy Markdown
Contributor

This PR add an option to all reaction models to filter out reactions where the product is also one of the reactants. This is hidden behind a flag remove_product_in_reactants, which is False by default for backwards compatibility.

@jla-gardner jla-gardner changed the title filter hallucinations filter reactions where one of the reactants is the product Feb 13, 2026
@jla-gardner

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree company="Microsoft"

@jla-gardner jla-gardner force-pushed the jla-gardner/remove-reactant-product-repetition branch from 529b0bd to 6aa1b26 Compare February 13, 2026 16:36

@AustinT AustinT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This makes sense to me. I vaguely recall thinking about this problem when initially implementing this library but forget why I didn't include a kwarg like this. Thanks for implementing!

@kmaziarz kmaziarz changed the title filter reactions where one of the reactants is the product Filter reactions where one of the reactants is the product Mar 3, 2026
@kmaziarz kmaziarz self-requested a review March 3, 2026 18:04

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

LGTM. Before merging, remember to update the CHANGELOG, and also fill in the PR description (see any other PR e.g. #144), which will then populate the commit message of the merge commit added to main 🙂

@kmaziarz kmaziarz force-pushed the jla-gardner/remove-reactant-product-repetition branch from 6aa1b26 to 0331b38 Compare March 30, 2026 14:00
@jla-gardner jla-gardner force-pushed the jla-gardner/remove-reactant-product-repetition branch from 0331b38 to f2fd57a Compare June 24, 2026 15:01
jla-gardner added a commit to jla-gardner/syntheseus that referenced this pull request Jun 24, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jla-gardner and others added 2 commits June 24, 2026 16:07
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jla-gardner jla-gardner force-pushed the jla-gardner/remove-reactant-product-repetition branch from 1b3531a to 8c85a89 Compare June 24, 2026 15:09
@kmaziarz kmaziarz merged commit 8bb9bb0 into microsoft:main Jun 25, 2026
10 checks passed
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.

4 participants