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

Breaking changes for MOI v0.10 #144

Closed
wants to merge 1 commit into from
Closed

Conversation

metab0t
Copy link
Contributor

@metab0t metab0t commented Sep 19, 2021

  • The new test/MathOptInterface/MOI_Wrapper.jl tests are all passed except some conflict tests that need further investigation
  • The old test/MathOptInterface/MOI_Wrapper.jl is renamed as test/MOI_Wrapper_legacy.jl to wipe out old MOI.Test related tests and keep handwritten tests. test/MathOptInterface/MOI_Wrapper.jl tests are all passed.
  • Some of test/MathOptInterface/MOI_callbacks.jl tests failed. @joaquimg Would you like to have a look?
  • I tried to solve Xpress ignore LP warm starts #127 by using XPRSaddmipsol to set up initial value of variables but I am not sure about the correctness.

@metab0t metab0t changed the title Break changes for MOI v0.10 Breaking changes for MOI v0.10 Sep 19, 2021
Copy link
Member

@odow odow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove the deprecated test file entirely.

It's okay to exclude the tests for now. They can be fixed in future PRs.

src/MOI/MOI_wrapper.jl Outdated Show resolved Hide resolved
src/MOI/MOI_wrapper.jl Outdated Show resolved Hide resolved
@joaquimg
Copy link
Member

Please do:

I tried to solve Xpress ignore warm starts #127 by using XPRSaddmipsol to set up initial value of variables but I am not sure about the correctness.

in a separate PR.

@joaquimg
Copy link
Member

@metab0t callbacks pass locally in master before you commits?

@metab0t
Copy link
Contributor Author

metab0t commented Sep 21, 2021

Please do:

I tried to solve Xpress ignore warm starts #127 by using XPRSaddmipsol to set up initial value of variables but I am not sure about the correctness.

in a separate PR.

Done.

@metab0t
Copy link
Contributor Author

metab0t commented Sep 21, 2021

@metab0t callbacks pass locally in master before you commits?

Yes, it passes before my commits.

@metab0t
Copy link
Contributor Author

metab0t commented Sep 21, 2021

The callback error
Snipaste_2021-09-21_23-22-52

@joaquimg
Copy link
Member

a few tests disappeared, can you review that?

@joaquimg
Copy link
Member

I noticed @odow asked for the removal of the old tests.
Are those corner cases covered in the default MOI tests?

@odow
Copy link
Member

odow commented Sep 21, 2021

Ah sorry. To clarify: you can remove most of the deprecated test file, i.e., anything that called MOI.DeprecatedTest. If there were additional Xpress-specific tests, we should keep those.

@odow
Copy link
Member

odow commented Sep 21, 2021

@metab0t this would be easier if I gave you access to make a PR to the jump-dev remote so that CI could run.

Are you based at a university? Your GitHub profile is a little sparse...

@odow
Copy link
Member

odow commented Sep 21, 2021

@metab0t, I talked to @joaquimg and I've sent you an invite to be an outside collaborator. Use the access wisely :)

The easiest thing to do is start a new PR against the jump-dev remote (CI only runs if you have access to the secret to enable Xpress to download).

Let me know if you need help doing that.

@metab0t
Copy link
Contributor Author

metab0t commented Sep 22, 2021

Ah sorry. To clarify: you can remove most of the deprecated test file, i.e., anything that called MOI.DeprecatedTest. If there were additional Xpress-specific tests, we should keep those.

OK, I will add those additional tests.

@metab0t, I talked to @joaquimg and I've sent you an invite to be an outside collaborator. Use the access wisely :)

The easiest thing to do is start a new PR against the jump-dev remote (CI only runs if you have access to the secret to enable Xpress to download).

Let me know if you need help doing that.

Thanks! I have accepted the invitation. I will start a new PR today.

@metab0t this would be easier if I gave you access to make a PR to the jump-dev remote so that CI could run.

Are you based at a university? Your GitHub profile is a little sparse...

Yes, I am currently a PhD student.

@metab0t
Copy link
Contributor Author

metab0t commented Sep 22, 2021

New PR #146 is opened.

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

Successfully merging this pull request may close these issues.

3 participants