Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fix PiExpression division and multiplication arithmetic #549

Merged
merged 3 commits into from
Feb 12, 2024

Conversation

pehamTom
Copy link
Member

@pehamTom pehamTom commented Feb 12, 2024

Description

Fix Arithmetic Issues when dividing or multiplying a PiExpression by a double.

Partially solves #486

Checklist:

  • The pull request only contains commits that are related to it.
  • I have added appropriate tests and documentation.
  • I have made sure that all CI jobs on GitHub pass.
  • The pull request introduces no new warnings and follows the project's style guidelines.

Copy link

codecov bot commented Feb 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (55752ce) 91.0% compared to head (562ee29) 91.1%.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #549   +/-   ##
=====================================
  Coverage   91.0%   91.1%           
=====================================
  Files        131     131           
  Lines      13664   13667    +3     
  Branches    2150    2150           
=====================================
+ Hits       12444   12451    +7     
+ Misses      1220    1216    -4     
Flag Coverage Δ
cpp 90.7% <100.0%> (+<0.1%) ⬆️
python 99.5% <ø> (ø)
Files Coverage Δ
include/mqt-core/operations/Expression.hpp 92.8% <100.0%> (ø)

... and 2 files with indirect coverage changes

result.type = ConstEvalValue::Type::ConstBool;
break;
case BinaryExpression::Equal:
result.value = lhs == rhs;

Check notice

Code scanning / CodeQL

Equality test on floating-point values

Equality checks on floating point values can yield unexpected results.
result.type = ConstEvalValue::Type::ConstBool;
break;
case BinaryExpression::NotEqual:
result.value = lhs != rhs;

Check notice

Code scanning / CodeQL

Equality test on floating-point values

Equality checks on floating point values can yield unexpected results.
Token(const Kind k, const size_t l, const size_t c, std::string s)
: kind(k), line(l), col(c), endLine(l), endCol(c), str(std::move(s)) {}

static std::string kindToString(const Kind kind) {

Check warning

Code scanning / CodeQL

Poorly documented large function

Poorly documented function: fewer than 2% comments for a function of 263 lines.
@pehamTom pehamTom changed the base branch from xx-pm-yy-gates to main February 12, 2024 11:26
@burgholzer burgholzer added fix Fix for something that isn't working ZX Anything related to the ZX package c++ Anything related to C++ code labels Feb 12, 2024
@burgholzer burgholzer added this to the ZX Package Improvements milestone Feb 12, 2024
Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

LGTM! Many thanks.

Just for my understanding: this should fix all of the test failures in the other feature PR, right? i.e. we can close the linked issue after merging.

@burgholzer burgholzer changed the title Fix piexpr 🐛 Fix PiExpression division and multiplication arithmetic Feb 12, 2024
@burgholzer burgholzer merged commit af9d20a into main Feb 12, 2024
35 checks passed
@burgholzer burgholzer deleted the fix-piexpr branch February 12, 2024 13:52
burgholzer added a commit that referenced this pull request Feb 12, 2024
## Description

This PR adds the last remaining unsupported gates to the ZX library.
In the process, it also introduces a new convenience function for
integer division of parameter expressions that simplifies some code. In
conjunction with #549, this also fixes #486.

Fixes #343 

## Checklist:

<!---
This checklist serves as a reminder of a couple of things that ensure
your pull request will be merged swiftly.
-->

- [x] The pull request only contains commits that are related to it.
- [x] I have added appropriate tests and documentation.
- [x] I have made sure that all CI jobs on GitHub pass.
- [x] The pull request introduces no new warnings and follows the
project's style guidelines.

---------

Signed-off-by: burgholzer <burgholzer@me.com>
Co-authored-by: Tom Peham <pehamtom@gmx.at>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Anything related to C++ code fix Fix for something that isn't working ZX Anything related to the ZX package
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants