Closed odow closed 2 years ago
First of all, I apologise for this breakage, I did not think that the change would break anybody. My thinking for this was the following:
scs
, rather than the scs_init
, scs_solve
, scs_finish
methods. The reason I thought this was that before this change those three methods had no advantage over the single-shot, because the ability to reuse the workspace with updated b
, c
vectors was missing / incomplete. Indeed they weren't even documented on the website for that reason. I thought then I was safe to fix those methods to make them actually useful, then add them to the documentation.scs-python
project.It's clear that this understanding was wrong, can you enlighten me why point 1 and 3 are wrong (I presume you are using the three functions in point 2)? Once I understand the problems fully I will be able to ensure this doesn't happen again.
I thought that adding a new field to a struct would not break anything, because any new fields would simply be ignored by the interface
In Julia we have to maintain the Julia equivalent of the C struct so that when we pass it to C, Julia has allocated the correct amount of memory. Adding fields at the end can be okay, because then we'll just pass a pointer but only use the first bits that are allocated. The problem comes if we add new fields in the middle.
I thought all users were using the single-shot solve method scs
Ah, this is our fault then.
We were using scs_init
, scs_solve
, scs_finish
. In which case, you can just say that this was not a breaking change and SCS.jl was abusing the internal API of the solver. I think we've always(?) used these three functions in SCS.jl, so I never checked if there was a different way to do things.
Finally, even if it somehow broke an interface that the interface would be safely pinned to the latest working version
SCS.jl was not pinning the underlying C library. It had a compatibility bound on any 3.x.y version :) so when 3.2.0 came along it happily upgraded. We can pin going forward to be more conservative.
SCS.jl wrapps the C-structs, which from the viewpoint of @bodono were private; @odow I think just pinning the library to the exact version(s) which preserve the ABI is the best options.
Adding fields at the end can be okay, because then we'll just pass a pointer but only use the first bits that are allocated. The problem comes if we add new fields in the middle.
tbh. I think it's a poor incentive to keep those fields out of "logical" order ;)
I think we still are using scs_(init|solve|finish)
;) and if we want at some point users to be able to scs_update
we need to continue doing so.
I think just pinning the library to the exact version(s) which preserve the ABI is the best options.
Agreed.
I'll also try to keep an eye on the PRs here (I missed the v3.2 one), but @bodono perhaps you could tag me if you make any changes to the C API?
@bodono I also try to keep my eye on developments in scs but feel free to ping me as well.
Thanks for the information @odow and @kalmarek. I will definitely keep you in the loop for any other breaking changes, but hopefully that's very rare from now on.
One question - is there no way for Julia to just rely on the information supplied in the header file and create its own structs, rather than wrap the SCS structs? For example in a C application using SCS we would just have an #include scs.h
at the top, and if I add a field to ScsInfo it wouldn't break the downstream application since any new fields would just be ignored by the application. This is basically why it didn't break the python and matlab interfaces.
We can generate the Julia structs from the header files. But this is usually done in a static manner, rather than dynamically on the user's machine.
The main problem is that the Julia interface lives in SCS.jl, while the binary dependency lives in SCS_jll.jl. So it was possible to install the same version of SCS.jl with different versions of SCS_jll. But we've now fixed things so SCS.jl declares exactly what versions of SCS_jll.jl it knows how to support.
What is the policy on versioning and API breakages to the C API?
The change from 3.0 to 3.2 broke SCS on JuMP.
The main impacts were:
ScsInfo
: https://github.com/jump-dev/SCS.jl/pull/251/files#r816266207scs_solve
: https://github.com/jump-dev/SCS.jl/pull/251/files#r816266633Ideally, code that works for SCS 3.0.0 should continue to work for any SCS 3.x.y release, and any new features would be introduced via new functions or options.
cc @kalmarek