[Features] project_isometric and PolarNewton
#67
Conversation
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
|
Some random benchmark for julia> @benchmark left_polar!($Ac, ($W, $P)) setup=(copy!($Ac, $A))
BenchmarkTools.Trial: 24 samples with 1 evaluation per sample.
Range (min … max): 207.428 ms … 232.445 ms ┊ GC (min … max): 0.12% … 10.51%
Time (median): 208.516 ms ┊ GC (median): 0.11%
Time (mean ± σ): 211.400 ms ± 8.050 ms ┊ GC (mean ± σ): 1.53% ± 3.52%
█
▃█▆█▅▃▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▆ ▁
207 ms Histogram: frequency by time 232 ms <
Memory estimate: 53.57 MiB, allocs estimate: 130.
julia> @benchmark left_polar!($Ac, ($W, $P), PolarNewton()) setup=(copy!($Ac, $A))
BenchmarkTools.Trial: 45 samples with 1 evaluation per sample.
Range (min … max): 106.843 ms … 133.498 ms ┊ GC (min … max): 0.00% … 0.33%
Time (median): 112.492 ms ┊ GC (median): 0.41%
Time (mean ± σ): 112.160 ms ± 3.850 ms ┊ GC (mean ± σ): 0.39% ± 0.11%
▁ ▁█
▃▁▃█▃▆▁▅▅█▃▁█▅██▅▃▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▃ ▁
107 ms Histogram: frequency by time 133 ms <
Memory estimate: 38.79 MiB, allocs estimate: 121. |
|
Ok I managed to screw up this PR with git rebase on the current main. |
|
Ok, I managed to recover. So this now includes the If the tests pass, this is ready for a review. |
lkdvos
left a comment
lkdvos
left a comment
There was a problem hiding this comment.
Overall looks great to me. Left a couple small comments, but otherwise happy to merge.
| export project_hermitian, project_antihermitian, project_isometric | ||
| export project_hermitian!, project_antihermitian!, project_isometric! |
There was a problem hiding this comment.
I'm slightly unhappy with project_isometric vs isisometry. Is it at all reasonable to have project_isometry, or does that not really make sense? Happy to leave as is too, just noticing this here.
There was a problem hiding this comment.
That is a good catch. But I would prefer isisometric, as all other checks are also using the adjective form, e.g. ishermitian, ...
I know this constitutes yet another breaking change 😄 .
I guess unitary is the odd duck, as that is both an adjective and a noun.
There was a problem hiding this comment.
In case a breaking change is not warranted, I guess we can take comfort in the fact that the list of is... functions in Base has both nouns and adjectives in there.
There was a problem hiding this comment.
For what it's worth, I slightly prefer isisometric and project_isometric (and I agree that it is nice if they match).
|
I addressed all the issues except for the |
|
Ok, I'll merge when the tests turn green. |
* Bump v0.6 * rename `gaugefix` -> `fixgauge` * reduce unnecessary warnings * fix `copy_input` signatures in Mooncake tests * Add changelog to docs
PolarNewton implementation. This is branched off and requires #64 .