Piracy in the StdLibs #30945
Comments
|
Wow, a couple interesting items here.
That's entirely in base, seems like an incorrect result. Pkg, are you kidding me? It doesn't even return a string...
Yep, should be moved to Base. Not sure It looks like every useful method of
|
|
You have the power to tick them (and other meanless ones and false positives) off right? |
|
That is really cool. I worry about about people getting too worried about piracy (there are occasionally some very good reasons for it), but having a tool to detect seems like a clear win. |
|
|
Right; I look forward to wearing my "I'm a type pirate" T-shirt :) I think this is best seen as a heuristic for finding misplaced code, unintentional method extensions, etc. |
This is kind of OK since it's the only way to make new irrational numbers. However, we should perhaps add a couple more like sqrt(2) to Base.
This is intentional, since we wanted to move doc viewing code out of Base. But I think it's still up in the air where all the Base.Docs code should go exactly.
Could maybe go in base, but VersionNumber isn't iterable. Maybe Pkg could arrange to wrap the argument in a VersionRange instead?
Can move to Base.
This might actually make sense in Base as well. The code for writing is much simpler than the code for parsing. We should maybe even use this as the output format of |
|
Fix at JuliaLang/Pkg.jl#1036 |
|
|
There's also julia/stdlib/SparseArrays/src/sparsevector.jl Line 1072 in 9bd5c18 SparseArrays seems like a chronic offender: #32213 |
|
This is missing e.g julia> @which rand(5,5) * rand(5,5)
*(A::AbstractArray{T,2} where T, B::AbstractArray{T,2} where T) in LinearAlgebra at ...julia/stdlib/v1.3/LinearAlgebra/src/matmul.jl:152 |
FWIW, type piracy means that it is impossible to look at the Project and know what stdlibs a package actually use. For example, you can do matrix multiplication or call |
|
#43127 fixes some of the sparse arrays related ones. |
|
Thoughts on a possible way forward here:
This is technically breaking since at the moment projects can do matmul without declaring a dependency on LinearAlgebra, so maybe we can make it a warning for a while. This is kind of just a lie because the code does already depend on LinearAlgebra, it just gets to pretend that it doesn't. |
|
It would be nice if this approach was made available for non-Stdlib packages as well. |
|
I think this was discussed in #51432. |
|
I think the main problem with that approach is that it continues to be brittle. Since it loads the package as long as any dependency declares it is in the manifest, changing dependency versions could still end up revealing it is broken (much the same as the current situation, where "that dependency" just happens to be the julia sysimg) |
|
Alternatively, Base Julia could ship with a simple libblastrampoline implementation that just does things like a 3 line matmul, so the definition is there and works. Using LinearAlgebra just triggers a BLAS load with better definitions. This would then making building without blas much easier. Then over time, using BLAS could just be using OpenBLAS, and BLAS could split from LinearAlgebra. In that sense, OpenBLAS is just an option, on no different footing than MKL. If people are so worried about performance we could just throw a warning the first time the generic fallback is hit with BLAS types recommending BLAS be loaded. But carrying BLAS around everywhere because of the possibility that someone might not know of the performance optimization seems like a heavy cost to pay. Usually signal and opt into performance. Note that removing BLAS from LinearAlgebra is probably considered breaking so that can't be without a 2.0. But adding methods so things work without BLAS wouldn't be broken? |
|
@ChrisRackauckas: I really don't think that shipping a subpar BLAS and constantly telling people not to use it is a viable path forward. @vtjnash: I don't understand you're comment about brittleness. You seem to be talking about some entirely different issue that afaict you haven't actually explained. The problem here is that we currently have to build LinearAlgebra into the sysimg and we'd like to not do that. Lazy JLL loading would help avoid always loading BLAS. Trimming can help prove that an application does not actually do any BLAS operations and produce a binary without it. Maybe that's enough. But it would be nice to be able to just look in the manifest for a project and know that it doesn't definitely doesn't need linear algebra and BLAS without doing any fancy static analysis. That's what my proposal does: if LinearAlgebra isn't in the project file, then a project cannot use LinearAlgebra and BLAS without failing, just like it would if someone tried to import LinearAlgebra; if LinearAlgebra isn't in the manifest for an application then it likewise definitely doesn't use LinearAlgebra and BLAS. There are two potential issues I can see with what I've proposed. First, that it's somewhat breaking. But I'd argue that in any code that this breaks, there was already a real but undeclared dependency on LinearAlgebra via piracy and that forcing that existing dependency to be explicitly declared is reasonable and something of a bug fix. The second issue is that it's weird for a method call to implicitly do an import. It's already possible with eval and that's basically what this would do, but it might be better to register it in some more visible way. |
|
Specifically this:
This is always true? Different versions of things can have different dependencies so I have no idea what makes this different. |
Should go full out then and require it to be a direct dependency of the package that uses it. But the whole automatically loading packages at run time has never worked that well. You easily run into world age errors etc. Also, the concern in #51432 (comment) needs to be addressed AFAIU. |
That may lead to a big issue in the long run. Particulary when one uses a sat solver that tends to blackout tracking with some logical processing That may be useful to design in term of cluster (a +/- open subgraph)
With special nodes meaning
With special links meaning
That may need a not so easy upgrade of the pkg solver. |
I'm really not sure we want to encourage this. If I had a time machine, I would maybe just change this aspect of the language. Certainly requiring people to do
Agree. That's what I'm suggesting.
I think @Keno's suggested approach there sounds great and matches very closely what I was thinking. [Aside: It's kind of annoying that this conversation is spread across so many issues and PRs.] To be clear, I suspect you cannot do this correctly with |
The issue is sometimes it's natural to have packages do type piracy in filling in features. I.e. sometimes there's a "*Core" or "*Base" package to implement an interface for overloading but the default implementation is in the main package. It's possible that every instance of this can be replaced by extensions, since this pattern began before the creation of extensions. But extensions are rather limited for implementing new features, and currently do not support dependencies between different extensions. |
But that's not done via type piracy? |
We don't have any runtime way to specify this though: as long as LinAlg is any indirect dependency, the methods get added. To force direct dependency, we would have to export a different * from LinAlg (which has fallbacks to call Base) and becomes the default for packages that explicitly use LinAlg. That is fairly doable technically, but might be awkward in some situations also. Though doing that (some sort of overriding expert) might solve the issue with mul, and other similar methods (eg sparse vcat) where the overloads are currently are just pirating the Base method. |
Yes it is. Eg |
Okay, let me rephrase, "it |
|
What's the alternative then? It feels like the type piracy pattern used for AbstractFFts/FFTW.jl for exactly the same reason its used in StdLib/LinearAlgebra.jl, because it's a natural pattern. And so if the problem is the same, so should the solution? |
Uh oh!
There was an error while loading. Please reload this page.
I have been playing with the idea of automatically detecting type piracy.
https://discourse.julialang.org/t/pirate-hunter/20402
Here is the results of running it will the standard libraries loaded.
/julia/stdlib/v1.1/Pkg/src/Types.jl:1453
I'm not sure how many of these already have issues, eg #28234
and how many are harmless.
I am sure someone more knowledge-able than me can strike a bunch of them as not problematic.
But I figured I should share the list
The text was updated successfully, but these errors were encountered: