Opened 3 years ago

Last modified 3 years ago

#20656 needs_info enhancement

MixedIntegerLinearProgram: Remove _variables dictionary

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-7.3
Component: numerical Keywords: lp
Cc: nbruin, dimpase, vdelecroix, jdemeyer, chapoton Merged in:
Authors: Matthias Koeppe Reviewers:
Report Upstream: N/A Work issues:
Branch: u/mkoeppe/mixedintegerlinearprogram__remove__variables_dictionary (Commits) Commit: b7906ffd31c2007d81cb556d23b8b814d2faaebb
Dependencies: #20461, #20462 Stopgaps:

Description

The '_variables' attribute dictionary maps a "variable" (an "element" of a MIPVariable "dictionary") to an index in the backend. This is redundant because these "variables" are elements of type LinearFunction, and as such know their index in the backend already.

sage: p = MixedIntegerLinearProgram(solver='GLPK')
sage: pv = p.new_variable(nonnegative=True)
sage: pv[0]
x_0
sage: pv[77]
x_1
sage: pv[0].dict().keys()
[0]
sage: pv[77].dict().keys()
[1]

See also: #20461, #20602

Change History (4)

comment:1 Changed 3 years ago by mkoeppe

  • Branch set to u/mkoeppe/mixedintegerlinearprogram__remove__variables_dictionary

comment:2 Changed 3 years ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Cc chapoton added
  • Commit set to b7906ffd31c2007d81cb556d23b8b814d2faaebb
  • Status changed from new to needs_review

New commits:

ca1298bMixedIntegerLinearProgram._linear_variable_index: New
b7906ffMixedIntegerLinearProgram: Remove _variables

comment:3 Changed 3 years ago by tscrim

  • Status changed from needs_review to needs_info

Needs a rebase. However, I'm worried this could cause a speed regression as a dict lookup is a system python call whereas you are now doing a number of python function calls. Also storing a (relatively?) small dict doesn't cost much memory I would think.

comment:4 Changed 3 years ago by mkoeppe

I haven't done any timings regarding this patch.

Note: See TracTickets for help on using tickets.