-
Notifications
You must be signed in to change notification settings - Fork 37
Implement default jac_lin* API to patch 0.22 #522
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
base: main
Are you sure you want to change the base?
Conversation
|
@dpo @amontoison A suggestion of patch for 0.22.1. |
|
I posted a message on Zulip to explain the situation. This PR will not help the current issue. |
|
@amontoison There is no need to alarm everyone. On Zulip, I'm quite sure nobody has any idea what you are talking about except for @tmigot and I. In addition, nobody there makes high-level decisions about JSO. Everybody (me first) appreciates your concerns to keep versions consistent and everything in sync. It looks like a mistake happened. Let's just try to fix it cleanly and constructively. After this, we will need to work together on procedures to make sure this doesn't happen again. When you write
you need to tell us why and be specific. I'm still trying to understand exactly what is causing problems and what isn't. |
|
@dpo It is just that I don't want the new release to land in the packages of I don't have full control on that. I am currently in New-Zealand with limited bandwidth but I will do my best to explain the issues asap. But it will give me a lot of work when I will be back to Chicago. We still have time to revert this mistake. |
|
Presumably, they test before merging. In the mean time, let's fix the issue. I'm also willing to help upgrading the various dependents here. I'm by no means asking you to do it all by yourself. @tmigot Says that he tested this PR against several packages. Now I'd like to understand why the breakage isn't working. |
I will explain the problem in more detail in #523. The core issue is not really code breakage, but rather version incompatibilities and the way the package manager handles them. If we switch to NLPModels.jl 0.22 and only support that version (which is the recommended workflow for breaking releases, since CI tests only the latest version), then all new releases of this package will depend on For example, if a solver uses the linear API such as Because of this, we won't be able to solve problems in
The issue is not caused by the code itself. In fact, only 4 or 5 packages using the linear API are affected at the code level. The actual breakage comes from the version bump: since 0.22 is a new minor release, the package manager prevents testing against it unless we update each The number of direct and indirect dependent packages is quite large. Even if we make 0.22.1 non-breaking at the code level, it still leads to incompatibility with the package manager. |
A suggestion of changes to make 0.22 less breaking.
I tested NLPModelsModifiers, NLPModelsTest, QuadraticModels, ADNLPModels .
Changes are very minor, for instance for NLPModelsModifiers and NLPModelsTest:
QuadraticModels and ADNLPModels were not requiring any modification.
I commented out the warn messages in some places, because it allocates.