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

Add solar.py back to TESTS #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add solar.py back to TESTS #34

wants to merge 1 commit into from

Conversation

bqpd
Copy link
Contributor

@bqpd bqpd commented Apr 28, 2018

Reverts #33

@mjburton11 It only fails on the Npods=5 solve on those machines, so in hindsight I could have commented that out instead of removing the whole file from TESTS.

@bqpd bqpd requested a review from mjburton11 April 28, 2018 23:58
@bqpd
Copy link
Contributor Author

bqpd commented Apr 29, 2018

retest this please

@mjburton11
Copy link

That's interesting that some machines are not robust enough to handle it

@bqpd
Copy link
Contributor Author

bqpd commented Apr 29, 2018

@galbramc, analysis of this issue has moved to this thread

@galbramc
Copy link

What has moved here?

@bqpd
Copy link
Contributor Author

bqpd commented Apr 29, 2018

Oh, discussion from convexengineering/gpkit#1306

Is it macys_VM, reynolds, and windows7x64 that have MOSEK 7? The new QPROP model in solar.py doesn't solve on those three machines. :-/

@galbramc
Copy link

Yeah those are the machines with mosek 7. Can you download 7 and give it a try on your machine? Do they still support 7?

@bqpd
Copy link
Contributor Author

bqpd commented Apr 30, 2018

it's still available for download but does not seem to be actively supported

@galbramc
Copy link

Your call then if you support 7...

@galbramc
Copy link

Don't forget that some people deal with locked down systems. Working with government agencies in particular have lots or restrictions on what they can put on their computer, and upgrading for 7 to 8 may be non-trivial.

@mjburton11
Copy link

test this please

@bqpd
Copy link
Contributor Author

bqpd commented May 8, 2018

@galbramc after thinking about it, and encountering another MOSEK 7/8 difference on #1315, I think we want to have MOSEK 8 on all machines.

When that's done solar.py should be able to pass tests on those three machines again and we should be able to merge this.

@bqpd
Copy link
Contributor Author

bqpd commented May 8, 2018

test this please

@bqpd
Copy link
Contributor Author

bqpd commented May 8, 2018

@mjburton11 there appears to be a bug on this branch? it's not passing on any of the machines.

@galbramc
Copy link

galbramc commented May 8, 2018

Can you do anything in your build script to check that people are only using mosek 8 or newer in that case? I don't know when 9 will come out, but I'm sure it will some day.

@galbramc
Copy link

galbramc commented May 8, 2018

I'm going to wait until the bug on this branch is fixed to double confirm that you want the switch to mosek 8 on all machines.

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

Successfully merging this pull request may close these issues.

3 participants