search

bradfordboyle / pyglpk

Updated fork of T. Finley's PyGLPK module
GNU General Public License v3.0
14 stars 11 forks source link

Setting env.mem_limit=0 now equivalent to env.mem_limit=None #1

Closed snorfalorpagus closed 9 years ago

snorfalorpagus commented 9 years ago

According to MemoryTestCase.testMemLimit, setting env.mem_limit=0 is valid.

bradfordboyle commented 9 years ago

Originally, I believe that the underlying GLPK function for setting memory accepted a value of zero for maxing the memory. Since at least GLPK 4.52, the argument to glp_mem_limit() must be positive---passing in zero is fatal and terminates execution.

I decided that the test case as originally written was incorrect, and disallowed setting env.mem_limit=0 (see debeef4 on the dev branch). Looking at the code for glp_mem_limit(), any value greater than or equal to 4096 results in the same limit being put in place.

My original intent was to mirror the GLPK interface, but Environment_setmemlimit() could be changed to allow env.mem_limit=0 as a shortcut for env.mem_limit = 4096. Thoughts?

snorfalorpagus commented 9 years ago

My opinion is that we should preserve the API established by PyGLPK, so that any existing code is not broken by the change. I think that env.mem_limit=0 should be equivalent to whatever env.mem_limit=None does (which I think this commit achieves). Perhaps we could raise a deprecation warning?

bradfordboyle commented 9 years ago

I've merged your changes into the dev branch and updated the tests for the new behavior. If everything looks good to you, I'll merge it into master.

snorfalorpagus commented 9 years ago

@bradfordboyle Looks good to me.