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

GetAllDimensionNames and GetDimensions are not iterable (python) #2453

Closed
ecotner opened this issue Mar 21, 2021 · 5 comments
Closed

GetAllDimensionNames and GetDimensions are not iterable (python) #2453

ecotner opened this issue Mar 21, 2021 · 5 comments
Assignees
Labels
Bug Lang: Python Python wrapper issue Solver: Routing Uses the Routing library and the original CP solver
Projects
Milestone

Comments

@ecotner
Copy link

ecotner commented Mar 21, 2021

What version of OR-Tools and what language are you using?
Version: 8.1.8487
Language: Python

Which solver are you using (e.g. CP-SAT, Routing Solver, GLOP, BOP, Gurobi)
Routing Solver

What operating system (Linux, Windows, ...) and version?
Ubuntu 20.04 (via Windows Subsystem for Linux v2)

What did you do?
I have created a small script that reproduces the error:

from ortools.constraint_solver import pywrapcp

manager = pywrapcp.RoutingIndexManager(10, 2, 0)
routing = pywrapcp.RoutingModel(manager)

def some_callback(index):
    return 1

some_callback_index = routing.RegisterUnaryTransitCallback(some_callback)

routing.AddDimension(
    some_callback_index,
    slack_max=0,
    capacity=2**32 - 1,
    fix_start_cumul_to_zero=True,
    name="some_dimension"
)

names = routing.GetAllDimensionNames()
print(names) # prints `<Swig Object of type 'std::vector< std::string > *' at 0x7f05e4142ae0>`
print(dir(names)) # prints a list of attributes, none of which seem relevant for iteration/indexing
len(names) # raises TypeError
names[0] # raises TypeError
next(names) # raises TypeError

The same errors occur when you swap GetAllDimensionNames for GetDimensions as well.

What did you expect to see
I would expect these methods to either return a python list containing the dimension names/dimension objects, or some other container with __iter__ and __getitem__ methods so that the container can be iterated over or indexed into to retrieve the elements. It looks like someone also ran into this issue in #1417, but it was closed with the recommendation that the user should keep track of the dimensions themselves. I think it would be very convenient to have the ability to inspect the routing model itself rather than having to handle it manually!

What did you see instead?
I observed the TypeErrors mentioned in the comments of the script.

Make sure you include information that can help us debug (full error message, model Proto).

Anything else we should know about your project / environment

@Mizux Mizux self-assigned this Mar 21, 2021
@Mizux Mizux added Bug Lang: Python Python wrapper issue Solver: Routing Uses the Routing library and the original CP solver labels Mar 21, 2021
@Mizux Mizux added this to To do in ToDo via automation Mar 21, 2021
@Mizux Mizux added this to the v8.3 milestone Mar 21, 2021
@Mizux
Copy link
Collaborator

Mizux commented Mar 21, 2021

Side note for myself: We should take a look at these lines

PY_LIST_INPUT_OUTPUT_TYPEMAP(int, PyInt_Check, PyInt_FromLong);
PY_LIST_INPUT_OUTPUT_TYPEMAP(int64, SwigPyIntOrLong_Check, PyLong_FromLongLong);
PY_LIST_INPUT_OUTPUT_TYPEMAP(double, PyFloat_Check, PyFloat_FromDouble);

and see if we can add a std::vector<string>

note: Should also check Java and .Net wrappers

@Mizux Mizux moved this from To do to In progress in ToDo Mar 23, 2021
@Mizux
Copy link
Collaborator

Mizux commented Mar 23, 2021

Test

  print(f'list: {routing.GetAllDimensionNames()}')
  print(f'type: {dir(routing.GetAllDimensionNames())}')

current:

swig/python detected a memory leak of type 'std::vector< std::string > *', no destructor found.
list: <Swig Object of type 'std::vector< std::string > *' at 0x7f23b61d7ba0>
swig/python detected a memory leak of type 'std::vector< std::string > *', no destructor found.
type: ['__class__', '__delattr__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__int__', '__le__', '__lt__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', 'acquire', 'append', 'disown', 'next', 'own']

@Mizux Mizux closed this as completed in 9d5fd80 Mar 23, 2021
ToDo automation moved this from In progress to Done Mar 23, 2021
@Mizux
Copy link
Collaborator

Mizux commented Mar 23, 2021

now:

list: ['Distance', 'Time']
type: ['__add__', '__class__', '__class_getitem__', '__contains__', '__delattr__', '__delitem__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__getitem__', '__gt__', '__hash__', '__iadd__', '__imul__', '__init__', '__init_subclass__', '__iter__', '__le__', '__len__', '__lt__', '__mul__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__reversed__', '__rmul__', '__setattr__', '__setitem__', '__sizeof__', '__str__', '__subclasshook__', 'append', 'clear', 'copy', 'count', 'extend', 'index', 'insert', 'pop', 'remove', 'reverse', 'sort']

@ecotner
Copy link
Author

ecotner commented Mar 25, 2021

thank you @Mizux! Is this change pushed to pypi, or do I need to clone/build the master branch to be able to use the fix?

@Mizux
Copy link
Collaborator

Mizux commented Mar 25, 2021

master only, we hope to release a v9.0 ASAP once last issues are fixed (or postponed)
see: https://github.com/google/or-tools/milestone/17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Lang: Python Python wrapper issue Solver: Routing Uses the Routing library and the original CP solver
Projects
ToDo
  
Done
Development

No branches or pull requests

2 participants