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

New package: ExpmvCore v0.1.0 #116607

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JuliaRegistrator
Copy link
Contributor

UUID: 08d6d8b8-3c9c-4c77-8a4f-ab2df6813922
Repo: https://github.com/jecs/StaticExpmv.jl.git
Tree: e5c90f2c671282d545834e7551e6e29e85a33949

Registrator tree SHA: 191228b6dd8b9d0e2965ae3e705fe54c51dcfee8
Copy link
Contributor

github-actions bot commented Oct 4, 2024

Hello, I am an automated registration bot. I help manage the registration process by checking your registration against a set of AutoMerge guidelines. If all these guidelines are met, this pull request will be merged automatically, completing your registration. It is strongly recommended to follow the guidelines, since otherwise the pull request needs to be manually reviewed and merged by a human.

1. New package registration

Please make sure that you have read the package naming guidelines.

2. AutoMerge Guidelines are all met! ✅

Your new package registration met all of the guidelines for auto-merging and is scheduled to be merged when the mandatory waiting period (3 days) has elapsed.

3. To pause or stop registration

If you want to prevent this pull request from being auto-merged, simply leave a comment. If you want to post a comment without blocking auto-merging, you must include the text [noblock] in your comment.

Tip: You can edit blocking comments to add [noblock] in order to unblock auto-merging.

@goerz
Copy link
Member

goerz commented Oct 4, 2024

#116605 (comment) still applies

@jecs
Copy link

jecs commented Oct 4, 2024

I would prefer to make this package available now so that my research collaborators can use it while I figure out how to integrate my code cleanly into the SciML package. I would need to look into how extensions, weak dependencies, and all of that work first. I do also see the value in having a standalone package that would result in lower precompilation times for code that deals exclusively with StaticArrays.

I would also prefer that whether my code is available publicly not depend on whether I want to incorporate my code into a larger package. What if I don't agree with the design choices in the package to which you want me to merge my code? What if I want to retain ownership of the project? I'm surprised to see this sort of strong-arming in the Julia community.

@goerz
Copy link
Member

goerz commented Oct 4, 2024

I would prefer to make this package available now so that my research collaborators can use it while I figure out how to integrate my code cleanly into the SciML package

Since registrations in General are permanent, it doesn't make sense to register it now and then later have it merged into another package. So, I'd want to hold off on merging this while you explore your options for contributing to the existing package.

I would also prefer that whether my code is available publicly not depend on whether I want to incorporate my code into a larger package.

Your code does not have to be registered in General for it to be "public". Your collaborators can install the package directly from GitHub with

] add https://github.com/jecs/StaticExpmv.jl

Or, you can register it in another registry, i.e., set up a LocalRegistry, to make it easier to work with the package if it is a dependency for other (unregistered) packages currently under development.

[Edit: that would install StaticExpmv, not the sub-package ExpmvCore. Sub-packages make this a little more complicated, and I'd have to figure out what the correct invocation of Pkg is]

@goerz
Copy link
Member

goerz commented Oct 4, 2024

I'm also a little confused about the package name and organization, in terms of this being a sub-package of https://github.com/jecs/StaticExpmv.jl.

The parent package StaticExpmv isn't registered, right? I don't think this has come up before, but we'd have to think a bit about whether it makes sense / is okay for a sub-package to be registered when the parent package is unregistered.

There's also that if the parent package is named StaticExpmv, should be the sub-package be StaticExpmvCore? There is already https://github.com/marcusps/ExpmV.jl (albeit unregistered), so @marcusps would have a claim on the name ExpmVCore.

Lastly, even for sub-packages, there has to be an independent README (in https://github.com/jecs/StaticExpmv.jl/tree/master/ExpmvCore) that explains the context and the relationship to the parent package, and possibly points to more complete documentation for the functionality implemented by the sub-package.

@jecs
Copy link

jecs commented Oct 4, 2024

Registering the main package requires registering the subpackage. That's why I'm starting with ExpmvCore before adding StaticExpmv. Otherwise the registration process errors out.

@goerz
Copy link
Member

goerz commented Oct 4, 2024

Alright, that makes sense. The sub-package should definitely be StaticExpmvCore, then, however.

Registering the main package requires registering the subpackage.

It would be possible (and even preferable, albeit a little more difficult) to manually prepare a PR that registers both StaticExpmv and StaticExpmvCore at the same time. I think to prepare such PRs, you also use the LocalRegistry package. Not 100% sure, it might be worth searching/asking on Discourse about the best workflow.

I'm not fundamentally opposed to having an independent StaticExpmv, if it specifically exploits properties of StaticMatrix in a way that does not fit into the philosophy of ExponentialUtilities. The relationship to ExponentialUtilities should definitely be clarified before the registration, though. I suppose @ChrisRackauckas directly is the person maintaining ExponentialUtilities

@gdalle
Copy link
Contributor

gdalle commented Oct 4, 2024

I personally think making code available is not a sufficient reason for registration. As soon as your package is on GitHub, it is installable and your collaborators can use it. The real question is whether your design philosophy is so different from what came before that it warrants a separate name in the general registry.
This is not strong arming by the way. I don't have any particular rights on the registry. Anyone in the community is free to object and discuss registrations, which is both cool and frustrating at times

@ChrisRackauckas
Copy link
Member

I'd be happy to pull this in as a static array extension. Though it is a bit of an odd idea because expmv tends to only do well for sufficiently large matrices (generating the exponential is pretty good for small matrices but has poor scaling) so I'm very curious if this is even a good idea in the first place. I'd be interested to see what the benchmarks say.

@jecs
Copy link

jecs commented Oct 4, 2024

@ChrisRackauckas Even for 6x6 matrices, I saw a speedup of 3x.

@jecs
Copy link

jecs commented Oct 4, 2024

@gdalle You're right. It's not strong arming. Just got a bit emotional about my code. Sorry.

@goerz I think that I will follow your advice and use LocalRegistry. Thanks for bearing with me.

@jecs
Copy link

jecs commented Oct 4, 2024

@ChrisRackauckas

julia> using StaticExpmv,StaticArrays,LinearAlgebra,BenchmarkTools

julia> A = I+SMatrix{6,6}(randn(6,6));x = SVector{6}(randn(6));

julia> @benchmark exp($A)*$x
BenchmarkTools.Trial: 10000 samples with 21 evaluations.
 Range (min … max):  960.286 ns …  1.708 μs  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     974.190 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   975.719 ns ± 27.055 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

        ▄ ▇ ▇ █▇ ▃                                              
  ▂▂▁▄▁▇█▁█▁█▁██▁█▁▆▄▁▃▁▂▁▂▂▁▁▁▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▂▂▁▂▁▂▁▂▂▁▂▁▂▂ ▃
  960 ns          Histogram: frequency by time         1.03 μs <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark expmv($A,$x)
BenchmarkTools.Trial: 10000 samples with 239 evaluations.
 Range (min … max):  313.636 ns … 552.996 ns  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     315.728 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   320.862 ns ±  17.998 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

  ▆█▁      ▂ ▃▁                                                 ▁
  ███▅▇▇▆▅▅████▆▆▅▅▅▅▅▅▅▅▄▅▅▅▆▆▅▅▅▅▅▅▅▅▅▅▅▅▅▄▄▃▄▅▅▅▆▅▅▆▆▆▅▅▅▅▄▅ █
  314 ns        Histogram: log(frequency) by time        414 ns <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> A = I+SMatrix{9,9}(randn(9,9));x = SVector{9}(randn(9));

julia> @benchmark exp($A)*$x
BenchmarkTools.Trial: 10000 samples with 8 evaluations.
 Range (min … max):  3.911 μs …   7.922 μs  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     4.062 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   4.104 μs ± 164.296 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

         █▁                                                    
  ▂▂▂▂▂▂▅██▆▃▂▄▆▄▄▃▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▁▂▂▂▂▂▂▂▂▂▂▂▂▂▂ ▂
  3.91 μs         Histogram: frequency by time        4.95 μs <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark expmv($A,$x)
BenchmarkTools.Trial: 10000 samples with 194 evaluations.
 Range (min … max):  500.216 ns … 975.304 ns  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     502.144 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   504.040 ns ±   9.893 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

  ▃██▇▂                  ▁▃▃                                    ▂
  █████▆▄▄▃▃▃▄▅▆▇▆▆▄▁▄▁▁▃███▇▅▄▄▅▃▅▁▄▆▇█▇▅▃▃▁▄▃▃▅▄▅▃▃▃▃▃▃▅▄▄▁▆▄ █
  500 ns        Histogram: log(frequency) by time        547 ns <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> A = I+SMatrix{12,12}(randn(12,12));x = SVector{12}(randn(12));

julia> @benchmark exp($A)*$x
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min … max):  25.208 μs … 125.375 μs  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     25.583 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   25.817 μs ±   1.497 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

   ▃██▂   ▃▅▂                                                  ▂
  ▅████▇▅████▆▄▄▄▅▅▄▆▅▅▅▄▃▅▅▅▅▄▅▆▇▇▅▃▄▅▅▄▆▅▅▅▄▅▆▅▅▆▆▅▄▁▄▄▄▅▄▄▆ █
  25.2 μs       Histogram: log(frequency) by time      32.1 μs <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark expmv($A,$x)
BenchmarkTools.Trial: 10000 samples with 10 evaluations.
 Range (min … max):  1.038 μs …  2.350 μs  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     1.050 μs              ┊ GC (median):    0.00%
 Time  (mean ± σ):   1.061 μs ± 41.686 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

           ▅    █                                 ▁           
  ▂▁▁▁▃▁▁▁▁█▁▁▁▁█▁▁▁▁▄▁▁▁▁▃▁▁▁▂▁▁▁▁▂▁▁▁▁▂▁▁▁▁▇▁▁▁▁█▁▁▁▁▃▁▁▁▃ ▂
  1.04 μs        Histogram: frequency by time        1.09 μs <

 Memory estimate: 0 bytes, allocs estimate: 0.

@ChrisRackauckas
Copy link
Member

Those are pretty good results. Unexpected. I can see that coming in handy for Magnus methods.

@jecs
Copy link

jecs commented Oct 5, 2024

Cool. Please let me know how you'd like to proceed. Whatever package you think this would fit nicely in, I'd be happy to incorporate this code in it.

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

Successfully merging this pull request may close these issues.

6 participants