convexengineering / gpkit

Geometric programming for engineers
http://gpkit.readthedocs.org
MIT License
206 stars 40 forks source link

Possible fix for your broken unittests #1573

Open tblanarik opened 1 year ago

tblanarik commented 1 year ago

I don't know if you've come across this yet, but I think the following breaking change to pint is also responsible for your broken unittests:

You can be reasonably sure because the last test that succeeds (794) shows:

while the first to break shows:

In addition, the stacktrace shows this is happening while unpickling solar_13.p:

  File "C:\Users\jenkins\workspace\CE_gpkit_Push_unit_tests\buildnode\windows10x64\optimizer\cvxopt\docs\source\examples\breakdowns.py", line 11, in <module>
    sol = pickle.load(open(dirpath+"solar_13.p", "rb"))
  File "C:\Users\jenkins\workspace\CE_gpkit_Push_unit_tests\buildnode\windows10x64\optimizer\cvxopt\venv_jenkins\lib\site-packages\pint\util.py", line 429, in __setstate__
    self._d, self._one, self._non_int_type = state
ValueError: too many values to unpack (expected 3)

I haven't had a chance to look around the project much yet (I just started by trying to run the unittests), but it seems like the fix is maybe just to update rtd_requirements.txt to read:

pint == 0.18
adce

If/when I am confident, I can also submit a PR to that effect.

tblanarik commented 1 year ago

What's strange is that the rest of the builds seem to be pinned at pint==0.9, which is why they haven't broken

eg: https://acdl.mit.edu/csi/view/convex%20engineering/job/CE_gpkit_Push_unit_tests/buildnode=macys_VM,optimizer=cvxopt/lastBuild/console

whoburg commented 1 year ago

@tblanarik! Great to hear from you! Thanks for the possible fix, great sleuthing. PR would be great.

@galbramc, the pin to pint==0.9 is interesting -- are we setting that in our unit testing scripts? All I see in setup.py is pint >= 0.8.1.

tblanarik commented 1 year ago

Yeah, I couldn't locate the pin of pint==0.9, and was wondering if it's managed outside of this repo by your Jenkins setup 🤷

galbramc commented 1 year ago

Hey guys! Yes the jenkins scripts were setup to install pint == 0.9. I've removed that now and Jenkins is installing pint-0.20.1. Hopefully you can resolve the issue now!

tblanarik commented 1 year ago

@galbramc - I wonder if that's actually going to break the rest of the tests 😆 , since pint 0.19.1 contains breaking changes.

Since I don't have access to the scripts, I'm wondering if you'd be willing to try pinning all of the Jenkins scripts to 0.18 and rerunning?

In my own fork, I ran your unittests using GitHub Actions, Python 3.9 (which I think your Jenkins is set to) and the following requirements:

pint==0.18
adce==1.3.3.2
cvxopt==1.3.0
numpy==1.23.1
plotly==5.9.0
scipy==1.8.1
matplotlib==3.5.2

and you can see the tests passing on both Ubuntu and Windows, using cvxopt at least (no mosek)

Actions screenshot CleanShot 2023-02-24 at 15 14 24@2x

Alternatively, is it reasonable to make the Jenkins script a part of the repo, to have more 👀 on it?

There must have been some difference between the scripts since only the windows10x64 builds failed.

CleanShot 2023-02-24 at 15 09 31@2x

Finally - perhaps you don't really mind if the unittests are broken and this is just noise to you. I can go away in that case 😆

galbramc commented 1 year ago

What is preventing us from fixing gpkit and/or the unit tests so it works with the latest pint?

whoburg commented 1 year ago

well, latest pint is currently broken per https://github.com/hgrecco/pint/issues/1507, right?

galbramc commented 1 year ago

Are we storing any pickles in the repo? Are pickles supposed to be loadable forever or simply be treated as temporary data? Do we just need to recreate the pickled files for the newer pint?

Btw, shouldn't you be getting ready for a ride on a rocket!

whoburg commented 1 year ago

ohhhh, good point. yep that's probably a simple fix.

And yes definitely - but I'm also on a campaign to get inbox zero before launch :)

galbramc commented 1 year ago

Inbox zero is far more challenging mission! My girlfriend and I would watch the launch if you had picked a more reasonable time of day :)

tblanarik commented 1 year ago

The question of loading pickles forever is where I was going to ask you guys what your approach was. If it's that you want GPKit to just always work with the latest releases of packages, awesome 👍. But if you wanted to maintain some kind of backwards compatibility for particular reasons, then this might be a more complicated fix.

Since I'm only just exploring this project for the first time, I don't know a lot about how people are using it / what sort of support for older packages you care to provide 😁