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

Improve photonics documentation #2502

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

Omar-ORCA
Copy link
Contributor

No description provided.

Copy link

copy-pr-bot bot commented Jan 10, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@schweitzpgi
Copy link
Collaborator

schweitzpgi commented Jan 10, 2025

/ok to test

Command Bot: Processing...

Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Jan 10, 2025
Signed-off-by: Omar Bacarreza <[email protected]>
@Omar-ORCA Omar-ORCA force-pushed the photonics_docs branch 2 times, most recently from a3177c8 to 44c9a4c Compare January 13, 2025 11:34
@Omar-ORCA Omar-ORCA marked this pull request as draft January 13, 2025 11:36
@Omar-ORCA Omar-ORCA marked this pull request as ready for review January 14, 2025 09:27
@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Jan 16, 2025

/ok to test

Command Bot: Processing...

@khalatepradnya khalatepradnya added the documentation Improvements or additions to documentation label Jan 16, 2025
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Jan 16, 2025
@khalatepradnya
Copy link
Collaborator

@mawolf2023 for reviewing documentation structure.

@mawolf2023
Copy link
Contributor

Wow, tricky to decide where to sort this in the docs. I could see a case made to list the photonics simulator with the other simulators. However, my gut thinks we group all of this new material under photonics, especially if it is an ORCA specific simulator. So in the reorganized backends , we have ORCA under the hardware->photonics tab. I say we let this photonics page introduce the orca simulator and show all of the examples. Then the example notebook in this PR could be transformed a bit into a simple application that shows the utility of the photonics simulator. That way it dose not get buried that CUDA-Q can now simulator photonics, but avoids confusing the backend or example sections. Thoughts @efratshabtai @khalatepradnya ?

@khalatepradnya
Copy link
Collaborator

Thanks, @Omar-ORCA , for the updates!
While you are at it, can you add the following patch?

diff --git a/docs/sphinx/targets/cpp/photonics_tbi_get_state.cpp b/docs/sphinx/targets/cpp/photonics_tbi_get_state.cpp
index 3b27be44f5..0354bd629c 100644
--- a/docs/sphinx/targets/cpp/photonics_tbi_get_state.cpp
+++ b/docs/sphinx/targets/cpp/photonics_tbi_get_state.cpp
@@ -1,6 +1,7 @@
 // Compile and run with:
 // ```
-// nvq++ --target orca-photonics photonics_tbi_get_state.cpp && ./a.out
+// nvq++ --library-mode --target orca-photonics photonics_tbi_get_state.cpp
+// ./a.out
 // ```

 #include <cudaq.h>
diff --git a/docs/sphinx/targets/cpp/photonics_tbi_sample.cpp b/docs/sphinx/targets/cpp/photonics_tbi_sample.cpp
index 90da940ec2..28055ec211 100644
--- a/docs/sphinx/targets/cpp/photonics_tbi_sample.cpp
+++ b/docs/sphinx/targets/cpp/photonics_tbi_sample.cpp
@@ -1,6 +1,7 @@
 // Compile and run with:
 // ```
-// nvq++ --target orca-photonics photonics_tbi_sample.cpp && ./a.out
+// nvq++ --library-mode --target orca-photonics photonics_tbi_sample.cpp
+// ./a.out
 // ```

 #include <cudaq.h>

With CUDA-Q v0.9.1, the library mode isn't default and must be explicitly set.

@khalatepradnya
Copy link
Collaborator

Wow, tricky to decide where to sort this in the docs. I could see a case made to list the photonics simulator with the other simulators. However, my gut thinks we group all of this new material under photonics, especially if it is an ORCA specific simulator. So in the reorganized backends , we have ORCA under the hardware->photonics tab. I say we let this photonics page introduce the orca simulator and show all of the examples. Then the example notebook in this PR could be transformed a bit into a simple application that shows the utility of the photonics simulator. That way it dose not get buried that CUDA-Q can now simulator photonics, but avoids confusing the backend or example sections. Thoughts @efratshabtai @khalatepradnya ?

Thanks, @mawolf2023 for the feedback. I think it may come to which PR gets merged first. Are you close to getting yours in?

@Omar-ORCA
Copy link
Contributor Author

Wow, tricky to decide where to sort this in the docs. I could see a case made to list the photonics simulator with the other simulators. However, my gut thinks we group all of this new material under photonics, especially if it is an ORCA specific simulator. So in the reorganized backends , we have ORCA under the hardware->photonics tab. I say we let this photonics page introduce the orca simulator and show all of the examples. Then the example notebook in this PR could be transformed a bit into a simple application that shows the utility of the photonics simulator. That way it dose not get buried that CUDA-Q can now simulator photonics, but avoids confusing the backend or example sections. Thoughts @efratshabtai @khalatepradnya ?

I just wanted to make clear that the photonics simulator is not ORCA specific, it can be used to simulate any photonic circuit.

@mawolf2023
Copy link
Contributor

@Omar-ORCA Thank you for clarifying.

In that case, @khalatepradnya and @efratshabtai, I think we add a tab in the SV simulator section for photonics and include all the examples here. Then, we could still update the applications section with a photonics application.

@khalatepradnya, I think the backend PR is nearly ready to merge, so probably this one can merge into that?

@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Jan 22, 2025

/ok to test

Command Bot: Processing...

Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Jan 22, 2025
@khalatepradnya khalatepradnya enabled auto-merge (squash) January 22, 2025 21:15
Signed-off-by: Pradnya Khalate <[email protected]>
@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Jan 24, 2025

/ok to test

Command Bot: Processing...

github-actions bot pushed a commit that referenced this pull request Jan 24, 2025
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants