feature: Support integer division with `//` operator

[daredevil3435]

Issue #, if available: #9

Description of changes: Added new file in operators, converters

Testing done:

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

• [x] I have read the CONTRIBUTING doc
• [x] I used the commit message format described in CONTRIBUTING
• [ ] I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

• [ ] I have added tests that prove my fix is effective or that my feature works (if appropriate)
• [ ] I have checked that my tests are not configured for a specific region or account (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[daredevil3435]

@rmshaffer closes #9

[daredevil3435]

Hi. I think due to bad weather my comments are not getting uploaded. so i am emailing you to see what I am doing wrong. all the things I have sent you are still showing pending.

On Fri, Jun 7, 2024 at 10:27 PM Ryan Shaffer @.***> wrote:

@.**** commented on this pull request.

In src/autoqasm/operators/arithmetics.py https://github.com/amazon-braket/autoqasm/pull/26#discussion_r1631474899 :

+# +# or in the "license" file accompanying this file. This file is +# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF +# ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. + +"""Operators for arithmetic operators: // """ + +from future import annotations + +from autoqasm import program +from autoqasm import types as aq_types + +from .utils import _register_and_convertparameters + +def fd(a: int, b: int) -> int | aq_types.IntVar:

you'll also want to add your new operator to the list of operators init.py:

https://github.com/amazon-braket/autoqasm/blob/main/src/autoqasm/operators/__init__.py

— Reply to this email directly, view it on GitHub https://github.com/amazon-braket/autoqasm/pull/26#pullrequestreview-2105009689, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUVFD563XNVXOYFVAF7YLUTZGHQ6XAVCNFSM6AAAAABI6RJQCWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMBVGAYDSNRYHE . You are receiving this because you were mentioned.Message ID: @.***>

[daredevil3435]

attaching scrrenshots.

On Fri, Jun 7, 2024 at 10:46 PM Kanak Umrani @.***> wrote:

Hi. I think due to bad weather my comments are not getting uploaded. so i am emailing you to see what I am doing wrong. all the things I have sent you are still showing pending.

On Fri, Jun 7, 2024 at 10:27 PM Ryan Shaffer @.***> wrote:

@.**** commented on this pull request.

In src/autoqasm/operators/arithmetics.py https://github.com/amazon-braket/autoqasm/pull/26#discussion_r1631474899 :

+# +# or in the "license" file accompanying this file. This file is +# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF +# ANY KIND, either express or implied. See the License for the specific +# language governing permissions and limitations under the License. + +"""Operators for arithmetic operators: // """ + +from future import annotations + +from autoqasm import program +from autoqasm import types as aq_types + +from .utils import _register_and_convertparameters + +def fd(a: int, b: int) -> int | aq_types.IntVar:

you'll also want to add your new operator to the list of operators init.py:

https://github.com/amazon-braket/autoqasm/blob/main/src/autoqasm/operators/__init__.py

— Reply to this email directly, view it on GitHub https://github.com/amazon-braket/autoqasm/pull/26#pullrequestreview-2105009689, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUVFD563XNVXOYFVAF7YLUTZGHQ6XAVCNFSM6AAAAABI6RJQCWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMBVGAYDSNRYHE . You are receiving this because you were mentioned.Message ID: @.***>

[rmshaffer]

attaching scrrenshots.

@daredevil3435 I'm unable to see the screenshots or any messages from you - can you try again?

[daredevil3435]

Hi. I don't what exactly is issue. Yesterday also I left multiple comments but all are still showing pending status. I think due to bad network that's happening. I am attaching screenshots of the errors for your reference.

On Sat, Jun 8, 2024 at 12:02 AM Ryan Shaffer @.***> wrote:

attaching scrrenshots.

@daredevil3435 https://github.com/daredevil3435 I'm unable to see the screenshots or any messages from you - can you try again?

— Reply to this email directly, view it on GitHub https://github.com/amazon-braket/autoqasm/pull/26#issuecomment-2155327920, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUVFD52MH3FTF2WDX6SHQO3ZGH4DZAVCNFSM6AAAAABI6RJQCWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJVGMZDOOJSGA . You are receiving this because you were mentioned.Message ID: @.***>

[daredevil3435]

ik it's weekend, but I have few questions to ask. I run tox unit test command and it gave me only 1 test failed error. plus the coverage report suggested that code in converters/arithmetics.py and ooperators/arithmetics.py has like only 35-40% coverage, mainly excluding all the main coding part. I am kind of confused about this now. Does that mean that my arithmetics code is wrong? or like my testing code is wrong?

[daredevil3435]

one more thing I observed is that if i do unit test for a specific function like tox -e unit-tests -- test_operators::test_comparison_gt tox -e unit-tests -- test_operators::test_comparison_lt
they are mostly failing.

[daredevil3435]

[daredevil3435]

[daredevil3435]

[daredevil3435]

After running more than 50 tests I am no longer overwhelmed with unit-tests. I think this PR or entire issue has taught me several new things!

[rmshaffer]

for this error message in your test screenshot:

TypeError: unsupported operand type(s) for //: 'IntVar' and 'IntVar'

This is great progress that you were able to get to this point! And here I think you are hitting a limitation of the underlying oqpy library. The base class OQPyExpression implements __truediv__ (see here) but does not implement __floordiv__ which is required to enable the // operator between variables. I will see if I can help with making the necessary changes in the oqpy library.

Can you please push the latest version of your code to this PR so that we can review the latest revisions? (It looks like the code is still from a few days ago.)

[daredevil3435]

for this error message in your test screenshot:

TypeError: unsupported operand type(s) for //: 'IntVar' and 'IntVar'

This is great progress that you were able to get to this point! And here I think you are hitting a limitation of the underlying oqpy library. The base class OQPyExpression implements __truediv__ (see here) but does not implement __floordiv__ which is required to enable the // operator between variables. I will see if I can help with making the necessary changes in the oqpy library.

Can you please push the latest version of your code to this PR so that we can review the latest revisions? (It looks like the code is still from a few days ago.)

Yes, working on it. This is why I actually tried out with normal integer division function as well. I also changed function arguments from int to Any as I thought maybe we're not accepting IntVar is the reason test is failing.

[rmshaffer]

the coverage report suggested that code in converters/arithmetics.py and ooperators/arithmetics.py has like only 35-40% coverage

Code coverage is indicating how much of your new code is actually executed when running the unit tests. So 35-40% of your code is being hit, and the rest is not being hit. It's a sign that you would want to write additional tests to cover all of the branches of your new code, to make sure it all runs as you intend it to.

Note that until the code is actually working (which will require the oqpy changes that I mentioned), you're unlikely to get to 100% because of the errors. So don't worry about the code coverage for now, until the code is fully working end-to-end.

[daredevil3435]

the coverage report suggested that code in converters/arithmetics.py and ooperators/arithmetics.py has like only 35-40% coverage

Code coverage is indicating how much of your new code is actually executed when running the unit tests. So 35-40% of your code is being hit, and the rest is not being hit. It's a sign that you would want to write additional tests to cover all of the branches of your new code, to make sure it all runs as you intend it to.

Note that until the code is actually working (which will require the oqpy changes that I mentioned), you're unlikely to get to 100% because of the errors. So don't worry about the code coverage for now, until the code is fully working end-to-end.

Thank you for saying this! I was worried if I have done mistakes in my coding hence failing tests.

[daredevil3435]

Now if you can see tox -e linters test is passed. in test_operators.py i have kept the code combinations I used for error handling. Once I have clean and correct testing function, I'll make the file neat just like before. I have kind of created mess in there. I'm really sorry about that. But I promise I'll make it nice once my PR is eligible for merging. @rmshaffer @laurencap

[daredevil3435]

the coverage report suggested that code in converters/arithmetics.py and ooperators/arithmetics.py has like only 35-40% coverage

Code coverage is indicating how much of your new code is actually executed when running the unit tests. So 35-40% of your code is being hit, and the rest is not being hit. It's a sign that you would want to write additional tests to cover all of the branches of your new code, to make sure it all runs as you intend it to.

Note that until the code is actually working (which will require the oqpy changes that I mentioned), you're unlikely to get to 100% because of the errors. So don't worry about the code coverage for now, until the code is fully working end-to-end.

After I updated my testing function (available rn in test_operators) , code coverage increased for converters/arithmetics and operators/arithmetics as 53 and 44 percent respectively.

[rmshaffer]

Closing this PR, as the changes here were incorporated into #29 and have been merged. Thank you again @daredevil3435 for picking this up!