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

[docs] API example for structured_state for upcoming release v0.6.0 #94

Merged
merged 10 commits into from
Apr 11, 2024

Conversation

PabloAndresCQ
Copy link
Collaborator

@PabloAndresCQ PabloAndresCQ commented Apr 3, 2024

Description

  • Updated examples/mps_tutorial.ipynb [view]
  • Added a new examples/ttn_tutorial.ipynb [view]

Checklist

  • I have run the tests on a machine with GPUs.
  • I have performed a self-review of my code.
  • I have commented hard-to-understand parts of my code.
  • I have made corresponding changes to the public API documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the changelog with any user-facing changes.

@PabloAndresCQ PabloAndresCQ requested a review from cqc-melf April 3, 2024 15:23
Copy link
Collaborator

@cqc-melf cqc-melf left a comment

Choose a reason for hiding this comment

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

Have you checked if everything is working?
Maybe it would be good to add the python only version from the notebooks as well?
Are you doing any CI checks for them?

@PabloAndresCQ
Copy link
Collaborator Author

Have you checked if everything is working?

I've checked the notebooks run fine on my personal laptop, which has NVIDIA GPUs.

Maybe it would be good to add the python only version from the notebooks as well?

Do you mean a version of the notebook without the result cells?

Are you doing any CI checks for them?

Our CI is very limited: since it's not setup on a computer with GPUs, CI does not run any automatic tests. This is far from ideal, and I'm hoping to get access to a cloud node with GPUs at some point in the future via AWS, or to set it up on a desktop the QEC team has given me access to. I'll work on this whenever some more people join the dev team.

@cqc-melf
Copy link
Collaborator

cqc-melf commented Apr 4, 2024

One of the problem with the ipynb files is that it is very difficult to compare them with older versions, we have solved this in the pytket examples in the way that we have the python version of the notebooks in the repo as well and check that they both match on the CI.
We do this with:

    # Check that notebook is generated from script:
    p2j -o -t ${name}-gen.ipynb python/${name}.py
    cmp ${name}.ipynb ${name}-gen.ipynb
    rm ${name}-gen.ipynb

See https://github.com/CQCL/pytket-docs/blob/main/examples/check-examples#L19

If you think this would be helpful here as well, I am happy to add this here to the PR if you want to :)

Not having a GPU on the CI is something we should try to solve maybe at a later point in time, I was not aware that is still missing...

@PabloAndresCQ
Copy link
Collaborator Author

Ah! That sounds nice indeed. I think it'd be a good addition to this PR, thanks! :)

@cqc-melf
Copy link
Collaborator

cqc-melf commented Apr 4, 2024

@PabloAndresCQ I have added that now, feel free to take a look :)

Copy link
Collaborator Author

@PabloAndresCQ PabloAndresCQ left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me.

I'll fix a small issue with the mps_tutorial notebook due to an attached picture being lost in the refactor, and then we're good to go.

Copy link
Collaborator Author

@PabloAndresCQ PabloAndresCQ left a comment

Choose a reason for hiding this comment

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

Done! I've also verified that .py files run with no issues on a computer with GPUs.

@PabloAndresCQ PabloAndresCQ requested a review from cqc-melf April 5, 2024 11:31
Copy link
Collaborator

@cqc-melf cqc-melf left a comment

Choose a reason for hiding this comment

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

Looks good, thank you for rescuing the image :)

@PabloAndresCQ PabloAndresCQ merged commit e0e4b5b into develop Apr 11, 2024
9 checks passed
@PabloAndresCQ PabloAndresCQ deleted the docs/v0.6.0 branch April 11, 2024 14:11
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.

2 participants