Skip to content
This repository was archived by the owner on Apr 9, 2024. It is now read-only.

chore: Miscellaneous refactor of CircuitSimplifier#307

Closed
TomAFrench wants to merge 6 commits intomasterfrom
refactor-simplifier
Closed

chore: Miscellaneous refactor of CircuitSimplifier#307
TomAFrench wants to merge 6 commits intomasterfrom
refactor-simplifier

Conversation

@TomAFrench
Copy link
Copy Markdown
Member

Related issue(s)

(If it does not already exist, first create a GitHub issue that describes the problem this Pull Request (PR) solves before creating the PR and link it here.)

Resolves (link to issue)

Description

Summary of changes

I noticed some code which doesn't do anything in CircuitSimplifier so I removed it. While I was here I updated simplify_arithmetic to use Expression methods so the logic reads closer to "plain english". The trivial case for simplify_quotient (where a == 0 || pred == 0 so we zero everything out) has also been moved up to the top of the if-else chain to get it out of the way sooner.

Dependency additions / changes

(If applicable.)

Test additions / changes

(If applicable.)

Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt with default settings.
  • I have linked this PR to the issue(s) that it resolves.
  • I have reviewed the changes on GitHub, line by line.
  • I have ensured all changes are covered in the description.

Additional context

(If applicable.)

@TomAFrench TomAFrench added this to the 0.15.0 milestone Jun 14, 2023
@TomAFrench
Copy link
Copy Markdown
Member Author

@guipublic @kevaundray Can I get a review on this so we can put this in 0.15.0?

@TomAFrench TomAFrench removed this from the 0.15.0 milestone Jun 14, 2023
@TomAFrench
Copy link
Copy Markdown
Member Author

Bumping to 0.16.0

@TomAFrench TomAFrench added this to the v0.16.0 milestone Jun 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants