Using Breakage CI for KeywordCalls

[cscherrer]

In case you haven't seen it, this is really helpful:
https://github.com/abelsiqueira/Breakage

It gives you results like this:
https://github.com/cscherrer/KeywordCalls.jl/actions/runs/888461853

For this to be really useful, tests in downstream packages (Transits and MeasureTheory) should be set up to confirm that functionality depending on KeywordCalls is working correctly.

Maybe this is already the case? If that's so, this can be closed right away. The hope is just to avoid having to bug you to check downstream functionality manually. Probably not such a big deal, since KeywordCalls is getting pretty stable now (other than some weirdness with default values). But it's still nice to have one more check on stability :)

[icweaver]

Oh that's really neat!

I just realized that I was using our old manual way of constructing objects in our tests instead of with KeywordsCalls. That should be switched over now (just a quick change), is there anything else that we should add on our end for Breakage to pick up on?

[cscherrer]

Yeah, Breakage is really nice. As far as anything else to add... I added some tests in KeywordCalls like this:

@test 0 == @ballocated f(alpha=1,b=2,c=3)

Maybe something like that (but building your objects that use KeywordCalls, not KeywordCalls directly), and one or two with @belapsed and some reasonable threshhold? That would make it easy to check that precompilation doesn't slow things down

[icweaver]

Great call (hah). I still need to add some extra tests to up our coverage in general, but the additional tests checking the memory allocation and elapsed time using KeywordCalls should be up now and look to be working well locally for me! I will be sure to give an update once the registry catches up with your latest changes

[icweaver]

Oh weird, it's not the registry, since cscherrer/cs-function-resolution was already merged before the update, IIUC. For some reason, I seem to be seeing allocations/redefinition warnings in the CI for Julia 1.5+ across platforms, even though KeywordCalls is not doing these things when I test it locally. Will double check the setup I am using

[cscherrer]

Good to know. I had been seeing these some of this in MeasureTheory too. Can you try with this?
https://github.com/cscherrer/KeywordCalls.jl/tree/cs-instance-type

It seemed better behaved to me, but I'm not sure it's yet 100%

[icweaver]

hmmm, still seems to have the same issue on the *nix platforms. Will check for Windows next

[cscherrer]

I think I need to learn how to use SnoopCompile. You ever tried it?
https://timholy.github.io/SnoopCompile.jl/stable/

[icweaver]

Same issue in Windows as well for Julia 1.5+. Was also able to reproduce locally for Julia 1.5 and nightly on linux. I guess 1.6 was our sweet spot, although it's a little strange that it only fails remotely

[icweaver]

Oh, I haven't, but maybe Miles has?

[cscherrer]

Oh 1.6 is ok? I was seeing trouble on 1.4 and 1.5 for a while now. Maybe I misread your "+"

[icweaver]

Right, I meant v1.5, 1.6, and nightly CI are failing for me, but 1.6 on my local machine seems to be ok at least (Linux Mint 20.1 x86_64). Strangely enough, v1.0 seems to be passing on all platforms

Oh! It is labeled as Julia 1 in the CI, but it is actually installing v1.6! Sorry about the confusion there. So really, it's just v1.5 and nightly that are failing there. At least that is lining up with what I am seeing locally now 😅

[cscherrer]

Oh good! That's consistent with what I was seeing. That's why I have this line in the tests:
https://github.com/cscherrer/KeywordCalls.jl/blob/cce1b93b913b92c8419eb52aa6b97e2cd44efa2f/test/runtests.jl#L61

[icweaver]

Oh wow, I did not realize non-allocating was specific to 1.6, adding that in now! Just out of curiosity, what feature does KeywordCalls use to accomplish this?

[cscherrer]

There's not a specific feature. It was just working great in 1.6 but not in earlier versions. I put the conditional in the tests to make sure we keep the 1.6 performance. It would be great is someone finds a way to make this better for 1.4-1.5 without compromising 1.6, but I don't think I'll spend more time on that until there's new information.

[icweaver]

I see, definitely reasonable!