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

Updated GCC 10.x to 11.x in CI tests #370

Merged
merged 5 commits into from
Oct 6, 2022
Merged

Updated GCC 10.x to 11.x in CI tests #370

merged 5 commits into from
Oct 6, 2022

Conversation

MonitSharma
Copy link
Contributor

@MonitSharma MonitSharma commented Oct 6, 2022

Before submitting

Please complete the following checklist when submitting a PR:

  • All new features must include a unit test.
    If you've fixed a bug or added code that should be tested, add a test to the
    tests directory!

  • All new functions and code must be clearly commented and documented.
    If you do make documentation changes, make sure that the docs build and
    render correctly by running make docs.

  • Ensure that the test suite passes, by running make test.

  • Add a new entry to the .github/CHANGELOG.md file, summarizing the
    change, and including a link back to the PR.

  • Ensure that code is properly formatted by running make format.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.


Context: Updated GCC 10.x to 11.x in CI tests

Description of the Change: Changed the version of the gcc and g++ from 10 to 11

Benefits: The update brings improved supports for newer C++ features.

Possible Drawbacks:

Related GitHub Issues: Issue

@codecov
Copy link

codecov bot commented Oct 6, 2022

Codecov Report

Merging #370 (4a3e047) into master (94d6571) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #370      +/-   ##
==========================================
- Coverage   99.88%   99.88%   -0.01%     
==========================================
  Files          49       49              
  Lines        4403     4401       -2     
==========================================
- Hits         4398     4396       -2     
  Misses          5        5              
Impacted Files Coverage Δ
pennylane_lightning/_version.py 100.00% <100.00%> (ø)
pennylane_lightning/src/simulator/KernelMap.hpp 99.03% <0.00%> (-0.01%) ⬇️
...ennylane_lightning/src/algorithms/JacobianTape.hpp 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mlxd
Copy link
Member

mlxd commented Oct 6, 2022

Hi @MonitSharma thank you for the PR.

Can you update the PR submission guidelines:

Context:

Description of the Change:

Benefits:

Possible Drawbacks:

Related GitHub Issues:

with the details of this work, and follow the check-boxes to ensure all requirements are met. Thank you.

You can also ignore the results from Update dev version automatically and codecov/project as we will handle those internally.

@MonitSharma
Copy link
Contributor Author

MonitSharma commented Oct 6, 2022

Before submitting

Please complete the following checklist when submitting a PR:

  • All new features must include a unit test.
    If you've fixed a bug or added code that should be tested, add a test to the
    tests directory!

  • All new functions and code must be clearly commented and documented.
    If you do make documentation changes, make sure that the docs build and
    render correctly by running make docs.

  • Ensure that the test suite passes, by running make test.

  • Add a new entry to the .github/CHANGELOG.md file, summarizing the
    change, and including a link back to the PR.

  • Ensure that code is properly formatted by running make format.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.


**Context:**Updated GCC 10.x to 11.x in CI tests

Description of the Change: Changed the version of the gcc and g++ from 10 to 11

Benefits: GCC can now auto-vectorize operations performing addition, subtraction, multiplication and the accumulate/subtract variants on complex numbers.

Possible Drawbacks:

Related GitHub Issues: none

@mlxd
Copy link
Member

mlxd commented Oct 6, 2022

Before submitting

Please complete the following checklist when submitting a PR:

* [x]  All new features must include a unit test.
  If you've fixed a bug or added code that should be tested, add a test to the
  [`tests`](../tests) directory!

* [x]  All new functions and code must be clearly commented and documented.
  If you do make documentation changes, make sure that the docs build and
  render correctly by running `make docs`.

* [x]  Ensure that the test suite passes, by running `make test`.

* [ ]  Add a new entry to the `.github/CHANGELOG.md` file, summarizing the
  change, and including a link back to the PR.

* [x]  Ensure that code is properly formatted by running `make format`.

When all the above are checked, delete everything above the dashed line and fill in the pull request template.

**Context:**Updated GCC 10.x to 11.x in CI tests

Description of the Change: Changed the version of the gcc and g++ from 10 to 11

Benefits: GCC can now auto-vectorize operations performing addition, subtraction, multiplication and the accumulate/subtract variants on complex numbers.

Possible Drawbacks:

Related GitHub Issues: none

Thanks again @MonitSharma . Can you edit this at the top of the PR (#370 (comment)). Also, you did not complete the Add a new entry to the .github/CHANGELOG.md file, summarizing the change, and including a link back to the PR. entry. Also for **Related GitHub Issues:** none can you include the URL of the original issue that inspired this PR.

For the listed comments: GCC can now auto-vectorize operations performing addition, subtraction, multiplication and the accumulate/subtract variants on complex numbers., can we verify this is the case for the updated compiler? If not, it should be fine to mention the update brings improved supports for newer C++ features.

Once this is done, we can start a review. Thanks!

@mlxd mlxd self-requested a review October 6, 2022 17:32
@AmintorDusko AmintorDusko self-requested a review October 6, 2022 17:33
Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Thanks for the effort @MonitSharma
Looks good to me! Happy to give it a first approval once the following action is complete:

I recommend that you Add a new entry to the .github/CHANGELOG.md file, summarizing the change, and including a link back to the PR.

We will not accept this as a finished PR until that step is complete :)

Copy link
Contributor

@AmintorDusko AmintorDusko left a comment

Choose a reason for hiding this comment

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

Hi @MonitSharma, nice work thanks.
You just need now to:
Add a new entry to the .github/CHANGELOG.md file, summarizing the change, and including a link back to the PR.

@mlxd mlxd mentioned this pull request Oct 6, 2022
5 tasks
@MonitSharma
Copy link
Contributor Author

I opened another PR for adding summary and my name in the CHANGELOG.md file. I am not sure whether that does it?

@mlxd
Copy link
Member

mlxd commented Oct 6, 2022

I opened another PR for adding summary and my name in the CHANGELOG.md file. I am not sure whether that does it?

You should be able to make an edit to the changelog in this branch, and have it included as part of this PR. If you are using the Github web interface, this is likely a little trickier than cloning the repository from your branch and editing it there.

However, you can use the in-browser VSCode editor by opening your branch at https://github.com/MonitSharma/pennylane-lightning/tree/patch-1 and hitting the . key. This should take you to an interface that looks like
image

You should be able to edit and save the changes in here into the same branch as your current work. Let me know if this helps!

@MonitSharma
Copy link
Contributor Author

Your message was really helpful, I added the details in the CHANGELOG.md file.

MonitSharma and others added 2 commits October 7, 2022 00:15
Co-authored-by: Amintor Dusko <[email protected]>
Co-authored-by: Amintor Dusko <[email protected]>
Copy link
Contributor

@AmintorDusko AmintorDusko left a comment

Choose a reason for hiding this comment

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

You addressed my questions and did a good job!
Thank you for that!

Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Manually updating internal CI dev trigger.

@mlxd
Copy link
Member

mlxd commented Oct 6, 2022

Great work @MonitSharma

We can call this as approved. We will handle the version string update (Update dev version automatically) and merge this work once we update it.

@MonitSharma
Copy link
Contributor Author

Thank You so much @mlxd and @AmintorDusko

@mlxd mlxd merged commit 2405af8 into PennyLaneAI:master Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants