-
Notifications
You must be signed in to change notification settings - Fork 15
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
Background Map for Plotting #170
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #170 +/- ##
==========================================
- Coverage 68.05% 66.51% -1.54%
==========================================
Files 41 43 +2
Lines 1709 1765 +56
==========================================
+ Hits 1163 1174 +11
- Misses 546 591 +45 ☔ View full report in Codecov by Sentry. |
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
|
CHANGELOG.md
Outdated
@@ -3,6 +3,7 @@ | |||
## v0.5.1-dev | |||
|
|||
- Add `classical_delay` and `quantum_delay` as keyword arguments to the `RegisterNet` constructor to set a default global network edge latency. | |||
- Implement a feature to include a map as a background for plotting (`generate_map`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a minor point on technical writing style: "implement a feature to do x" is a bit wordy. "implement" already implies there is a "feature" and vice-versa. Something that more immediately gets to "do x" reads more naturally. You would notice though that I do not always remember to fix this style issue in my own writing
@@ -1,24 +1,59 @@ | |||
"""Draw the given register network. | |||
|
|||
Requires a Makie backend be already imported.""" | |||
function registernetplot end | |||
function registernetplot(args...; kwargs...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This workaround of using get_extension
is something I set up for use with types and structs, but it is not a very good way to deal with functions, given the multimethod nature of julia.
Methods of functions can be added in extensions -- that would be much more natural than what we are doing here, of using a new function in a new namespace.
For now we can probably keep this implementation, but it would be important to create an issue to track fixing this hack.
Potentially a better way to do this would be to have a hint attached to method errors. Something like what is done in this PR https://github.com/QuantumSavory/QuantumClifford.jl/pull/416/files# but just warning that the method error happened because the necessary package was not imported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, it seems it is worse than that. Currently the way you have namespaced things it looks like a method is being overridden instead of a new function being made. That can be easily fixed by just restricting what is being imported in the namespace and keeping your current implementation.
But given that things need to be fixed anyway, I suggest doing the proper fix with method error hints that I described above.
Here is the errors I am currently getting
[ Info: Precompiling QuantumSavoryMakie [92a66160-645b-5231-a29a-c456122f462a] (cache misses: wrong dep version loaded (4), incompatible header (2))
WARNING: Method definition registernetplot(Any...) in module QuantumSavory at /home/stefan/Documents/ScratchSpace/quantumjulia/QuantumSavory.jl/src/plots.jl:4 overwritten in module QuantumSavoryMakie at /home/stefan/.julia/packages/MakieCore/EU17Y/src/recipes.jl:194.
ERROR: Method overwriting is not permitted during Module precompilation. Use `__precompile__(false)` to opt-out of precompilation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fleshing this out! Could you address the following two changes:
- add to the documentation page with pretty examples
- switch from using
get_extension
and a new function, to using the same function (by adding a new method to it from the package extensions) and having a method error hint in case the method does not exist yet (i.e. the Makie library is not imported yet)
Re-request a review when that is done
This PR is for a feature to add a map as a background for plotting.
The package
GeoMakie
was used, and the extensionQuantumSavoryGeoMakie
was created. Additionally, error-handling messages for extensions were implemented, as well as making plotting functions support different imputs.The new function
generate_map
outputs a Makie Axis, which integrates seamlessly withregisternetplot_axis
as before.Example usage:
This generates the following figure: