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

Fixing documentation in Glossary, Links in Examples and Resolving Issue #66 #68

Merged
merged 1 commit into from
Jul 6, 2024

Conversation

Fe-r-oz
Copy link
Contributor

@Fe-r-oz Fe-r-oz commented May 10, 2024

Hi, was reading the glossary and found a small typo. Fixed it.

@Fe-r-oz Fe-r-oz changed the title Fixing documentation in Glossary Fixing documentation in Glossary and Links in Examples May 10, 2024
@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented May 10, 2024

In the specific examples page, the examples links were leading to error pages:

https://juliaquantumcontrol.github.io/Krotov.jl/stable/examples/simple_state_to_state/ (leads to 404 error page)

Replaced all the links with the correct ones: For instance

https://juliaquantumcontrol.github.io/QuantumControlExamples.jl/stable/examples/simple_state_to_state/#Optimization-of-a-State-to-State-Transfer-in-a-Two-Level-System (leads to correct page)

@Fe-r-oz Fe-r-oz changed the title Fixing documentation in Glossary and Links in Examples Fixing documentation in Glossary and Links in Examples and Addressing Issue #66 May 10, 2024
@goerz goerz added the breaking PRs that break compatibility label May 10, 2024
@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented May 10, 2024

Some links and description pages in QuantumControlExampes.jl have to be modified as well.

  1. https://juliaquantumcontrol.github.io/QuantumControlExamples.jl/stable/tutorials/krotov_pulse_**parametrization**/#Pulse-Parametrization-for-Krotov%27s-Method

  2. https://juliaquantumcontrol.github.io/QuantumControlExamples.jl/stable/tutorials/krotov_pulse_**parametrization**/

I will change the spelling in the link to parameterization in Example Page of QuantumControl.jl for Pulse Parameterization example for Krotov Method. Later, after this PR, we can modify thekrotov.jlso that everything is consistent in all places.

https://juliaquantumcontrol.github.io/QuantumControlExamples.jl/stable/tutorials/krotov_pulse_**parameterization**/)

@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented May 10, 2024

Sorry for another workflow run! Now, I made the spelling in the links leading to QuantumControlExampes.jl examples consistent as well.

The pulse_parameterization and other tests except CI doctype all passed in first workflow run though.

@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented May 10, 2024

The CI doctype test failing because of extra line. Fixed it.

Also, improved the documentation in QuantumControlExamples.jl as well: JuliaQuantumControl/QuantumControlExamples.jl#1

@goerz
Copy link
Member

goerz commented May 12, 2024

Cool, thanks! Since this is a breaking change, I'll want to let it sit for a bit to accumulate some other breaking changes. In particular, I'm organizing a workshop in two weeks that will feature the QuantumControl package, and I'm not going to merge anything breaking before that.

I'll have a look at these PRs afterward.

docs/src/glossary.md Outdated Show resolved Hide resolved
@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Jun 12, 2024

Hi Michael,

Is this codestyle error simply occuring because we normalized the spelling of one word? In the main, src/ file, the evaluate function used which is same as shown via +, but the diff assumes it's the - minus one.

function evaluate(
deriv::ShapedParametrizationContinuousDerivative,
tlist,
n;
vals_dict=IdDict()
)

diff --git a/src/pulse_parameterizations.jl b/src/pulse_parameterizations.jl
│ index 09f5f1f..ebacbdf 100644
│ --- a/src/pulse_parameterizations.jl
│ +++ b/src/pulse_parameterizations.jl
│ @@ -400 +400,6 @@ end
│ -function evaluate(deriv::ShapedParameterizationPulseDerivative, tlist, n; vals_dict=IdDict())
│ +function evaluate(
│ +    deriv::ShapedParameterizationPulseDerivative,
│ +    tlist,
│ +    n;
│ +    vals_dict=IdDict()
│ +)
└ @ Main ~/work/_temp/[24](https://github.com/JuliaQuantumControl/QuantumControl.jl/actions/runs/9052722822/job/24928610529?pr=68#step:6:25)a1a795-44c6-4977-b0c9-9b0c8d967774:5
Error: Process completed with exit code 1.

Copy link
Member

@goerz goerz left a comment

Choose a reason for hiding this comment

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

I think that’s using the old misspelling (sorry, this is out of context, and no longer applicable. Disregard. I can't figure out how to delete the comment)

@goerz
Copy link
Member

goerz commented Jun 12, 2024

Is this codestyle error simply occuring because we normalized the spelling of one word?

Sure, there's a maximum line length, so if you add a letter to word that can push it over the limit, and the line has to be broken into multiple lines

@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Jun 12, 2024

if you add a letter to word that can push it over the limit, and the line has to be broken into multiple lines

I see, we can change the maximum line length by a couple letters so that the doc-style test passes?

I checked for any potential whitespaces, even used the same src file present in main, just changed ..ri.. to ..eri.. to be sure that there was not any potential whitespaces.

@goerz
Copy link
Member

goerz commented Jun 12, 2024

we can change the maximum line length

No, definitely not. Use make codestyle (or the equivalent command in the Julia REPL) to reformat the code to adhere to the required style.

@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Jun 12, 2024

Thanks! I'll explore make codestyle or alternative commands.

@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Jun 13, 2024

Applied make codestyle! Sorry, I didn't knew about the make stuff before!

I will now usetest()to see to make sure that all the tests pass!

julia --project=test -e 'using JuliaFormatter; format(".", verbose=true)'
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/devrepl.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/docs/generate_api.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/docs/make.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/ext/QuantumControlFiniteDifferencesExt.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/ext/QuantumControlZygoteExt.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/src/QuantumControl.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/src/deprecate.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/src/functionals.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/src/print_versions.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/src/pulse_parameterizations.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/src/reexport.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/src/set_default_ad_framework.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/src/workflows.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/test/clean.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/test/init.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/test/runtests.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/test/test_functionals.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/test/test_optimize_or_load.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/test/test_pulse_parameterizations.jl
Formatting /home/Desktop/New/qc/4/JuliaQuantumControl/QuantumControl.jl/test/test_run_or_load.jl

@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Jun 13, 2024

Test Results

Test Summary:           | Pass  Total  Time
QuantumControl versions |    3      3  1.9s

* Functionals (test_functionals.jl):
189.185000 seconds (73.64 M allocations: 4.808 GiB, 1.38% gc time, 46.03% compilation time: <1% of which was recompilation)

* Run-or-load (test_run_or_load.jl):
134.203390 seconds (21.57 M allocations: 1.472 GiB, 0.71% gc time, 38.25% compilation time: 41% of which was recompilation)

* Optimize-or-load (test_optimize_or_load.jl):

 17.979704 seconds (23.57 M allocations: 1.657 GiB, 5.28% gc time, 99.17% compilation time: 9% of which was recompilation)

* Pulse Parameterizations (test_pulse_parameterizations.jl):
 26.718649 seconds (23.18 M allocations: 1.617 GiB, 3.75% gc time, 84.87% compilation time: 14% of which was recompilation)

Test Summary:             | Pass  Total     Time
QuantumControl            |   96     96  6m08.3s
  Functionals             |   19     19  3m09.2s
  Run-or-load             |    5      5  2m14.2s
  Optimize-or-load        |   25     25    18.0s
  Pulse Parameterizations |   47     47    26.7s
368.754314 seconds (142.45 M allocations: 9.587 GiB, 1.49% gc time, 48.70% compilation time: 15% of which was recompilation)
┌───────────────────────────────────────────┬────────┬────────┬────────┬──────────┐
│ File name                                 │  Total │    Hit │ Missed │ Coverage │
├───────────────────────────────────────────┼────────┼────────┼────────┼──────────┤
│ ext/QuantumControlFiniteDifferencesExt.jl │     13 │     13 │      0 │     100% │
│ ext/QuantumControlZygoteExt.jl            │     12 │     12 │      0 │     100% │
│ src/QuantumControl.jl                     │      6 │      6 │      0 │     100% │
│ src/deprecate.jl                          │      0 │      0 │      0 │        - │
│ src/functionals.jl                        │     99 │     90 │      9 │      91% │
│ src/print_versions.jl                     │     15 │     14 │      1 │      93% │
│ src/pulse_parameterizations.jl            │    129 │     88 │     41 │      68% │
│ src/reexport.jl                           │      7 │      7 │      0 │     100% │
│ src/set_default_ad_framework.jl           │      7 │      6 │      1 │      86% │
│ src/workflows.jl                          │    151 │    135 │     16 │      89% │
├───────────────────────────────────────────┼────────┼────────┼────────┼──────────┤
│ TOTAL                                     │    439 │    371 │     68 │      85% │
└───────────────────────────────────────────┴────────┴────────┴────────┴──────────┘
nothing

@Fe-r-oz Fe-r-oz requested a review from goerz June 13, 2024 09:50
@Fe-r-oz Fe-r-oz changed the title Fixing documentation in Glossary and Links in Examples and Addressing Issue #66 Fixing documentation in Glossary, Links in Examples and Resolving Issue #66 Jun 13, 2024
@goerz goerz merged commit dca972c into JuliaQuantumControl:master Jul 6, 2024
3 checks passed
goerz added a commit to JuliaQuantumControl/QuantumControlExamples.jl that referenced this pull request Jul 6, 2024
@Fe-r-oz Fe-r-oz deleted the Glossary branch July 24, 2024 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking PRs that break compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants