ecole icon indicating copy to clipboard operation
ecole copied to clipboard

Too many indices!

Open AntoinePrv opened this issue 4 years ago • 0 comments

Describe the problem or improvement suggested

There are too many indices in Ecole, which is confusing at best, but most likely error prone.

  • NodeBipartite columns observation are indexed by the LP position of columns;
  • Khalil2016 are ordered by whatever PseudoBranchCands variables order is
  • Branching action set maps the order of PseudoBranchCands OR LPBranchCands variables to the LP position.
  • Branching action is to be an index of LP position

I find this all very confusing, and I spend quite some time on that code, actually I'm reading it right now. The indices and ordering is also global, so one cannot safely make changes somewhere without risking breaking everything. Furthermore, it is not possible to use Khalil2016 and Branching with LPBranchCands because we cannot recover that index.

This will only grow more complicated as we add observations/environments.

Describe the solution you would like

The ordering of variables or column should be self contained.

  • Observations should return an additional vector that tell for every line the SCIP_VAR* for the features;
  • Branching environment should only accept a SCIP_VAR*, no indices involved;
  • Branching action_set would need to become a vector of SCIP_VAR*, which NodeBipartite users would need to pass through numpy.argwhere to get the indices they had before.

Describe alternatives you have considered

Additional context

AntoinePrv avatar Feb 23 '21 17:02 AntoinePrv