search

VROOM-Project / pyvroom

Vehicle Routing Open-source Optimization Machine
BSD 2-Clause "Simplified" License
67 stars 14 forks source link

Upstream v1.14.0 #106

Closed SebMilardo closed 3 months ago

SebMilardo commented 4 months ago

Updating the version of vroom included in pyvroom. I'm creating a draft PR to keep track of the work done:

I'm currently getting the following error when running make test:

ImportError while loading conftest '/pyvroom/conftest.py'.
conftest.py:3: in <module>
    import vroom
src/vroom/__init__.py:5: in <module>
    from ._vroom import _main, JOB_TYPE, STEP_TYPE  # type: ignore
E   ImportError: /pyvroom/src/vroom/_vroom.cpython-312-x86_64-linux-gnu.so: undefined symbol: _ZTVN5vroom4cvrp6TSPFixE

I'll try to find a solution...

jonathf commented 4 months ago

Errors like that means you are missing a an include to some resource in vroom upstream.

Use a c++ demangler (many online if you search) to find the name, and grep/search for the name in vroom.

In simple cases a simple include is enough. In more complicated cases we might need to update the bind layer code in some way.

SebMilardo commented 4 months ago

Thanks a lot! I fixed the imports. Now I have 5 tests failing and 39 passed. I'll try to solve them by tomorrow

=================================================================================== test session starts ===================================================================================
platform linux -- Python 3.12.3, pytest-8.2.1, pluggy-1.5.0
rootdir: /pyvroom
configfile: pyproject.toml
collected 44 items                                                                                                                                                                        

README.rst F                                                                                                                                                                        [  2%]
test/input/test_vehicle_step.py ...                                                                                                                                                 [  9%]
test/test_amount.py .F.                                                                                                                                                             [ 15%]
test/test_break.py ..                                                                                                                                                               [ 20%]
test/test_file_handle.py ..                                                                                                                                                         [ 25%]
test/test_job.py ..                                                                                                                                                                 [ 29%]
test/test_libvroom_examples.py F                                                                                                                                                    [ 31%]
test/test_location.py ...                                                                                                                                                           [ 38%]
test/test_time_window.py ......                                                                                                                                                     [ 52%]
test/test_vehicle.py F                                                                                                                                                              [ 54%]
src/vroom/amount.py .                                                                                                                                                               [ 56%]
src/vroom/break_.py .                                                                                                                                                               [ 59%]
src/vroom/input/forced_service.py .                                                                                                                                                 [ 61%]
src/vroom/input/input.py .                                                                                                                                                          [ 63%]
src/vroom/input/vehicle_step.py .......                                                                                                                                             [ 79%]
src/vroom/job.py ...                                                                                                                                                                [ 86%]
src/vroom/location.py ...                                                                                                                                                           [ 93%]
src/vroom/time_window.py .                                                                                                                                                          [ 95%]
src/vroom/vehicle.py F.                                                                                                                                                             [100%]

======================================================================================== FAILURES =========================================================================================
__________________________________________________________________________________ [doctest] README.rst ___________________________________________________________________________________
052   ...                           vroom.Job(1515, location=1),
053   ...                           vroom.Job(1616, location=2),
054   ...                           vroom.Job(1717, location=3)])
055 
056   >>> solution = problem_instance.solve(exploration_level=5, nb_threads=4)
057 
058   >>> solution.summary.cost
059   6411
060 
061   >>> solution.routes.columns
UNEXPECTED EXCEPTION: RuntimeError('bad optional access')
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/doctest.py", line 1361, in __run
    exec(compile(example.source, filename, "single",
  File "<doctest README.rst[7]>", line 1, in <module>
  File "/pyvroom/src/vroom/solution/solution.py", line 62, in routes
    array = numpy.asarray(self._routes_numpy())
                          ^^^^^^^^^^^^^^^^^^^^
RuntimeError: bad optional access
/pyvroom/README.rst:61: UnexpectedException
__________________________________________________________________________________ test_amount_operator ___________________________________________________________________________________

    def test_amount_operator():
        amo1 = vroom.Amount([1, 2, 3])
        amo2 = vroom.Amount([2, 2, 3])

>       assert amo1 << amo2

test/test_amount.py:31: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = vroom.Amount([1, 2, 3]), other = vroom.Amount([2, 2, 3])

    def __lshift__(self, other: Amount) -> bool:
        other = Amount(other)
        if len(self) != len(other):
            raise _vroom.VroomInternalException("Comparing two Amount of different length")
>       return self._lshift(other)
E       AttributeError: 'Amount' object has no attribute '_lshift'. Did you mean: '__lshift__'?

src/vroom/amount.py:98: AttributeError
_____________________________________________________________________________ test_example_with_custom_matrix _____________________________________________________________________________

    def test_example_with_custom_matrix():
        problem_instance = vroom.Input()

        problem_instance.set_durations_matrix(
            profile="car",
            matrix_input=[[0, 2104, 197, 1299],
                          [2103, 0, 2255, 3152],
                          [197, 2256, 0, 1102],
                          [1299, 3153, 1102, 0]],
        )
        problem_instance.add_vehicle([vroom.Vehicle(7, start=0, end=0),
                                      vroom.Vehicle(8, start=2, end=2)])
        problem_instance.add_job([vroom.Job(id=1414, location=0),
                                  vroom.Job(id=1515, location=1),
                                  vroom.Job(id=1616, location=2),
                                  vroom.Job(id=1717, location=3)])
        solution = problem_instance.solve(
            exploration_level=5, nb_threads=4)

        assert solution.summary.cost == 6411
        assert solution.summary.unassigned == 0
        assert solution.unassigned == []

>       routes = solution.routes

test/test_libvroom_examples.py:31: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <vroom.solution.solution.Solution object at 0x7f18ef75f7d0>

    @property
    def routes(self) -> pandas.DataFrame:
        """
        Frame outlining all routes for all vehicles.

        It includes the following columns.

        vehicle_id:
            Id of the vehicle assigned to this route.
        type:
            The activity the vehicle is performing. The available
            categories are `start`, `end`, `break`, `job`, `delivery`
            and `pickup`.
        arrival:
            Timepoint when the actvity ends.
        duration:
            The length of the activity.
        setup:
            Total setup time for this route.
        service:
            Total service time for this route.
        waiting_time:
            Total waiting time for this route.
        location_index:
            The index for the location of the destination.
        longitude:
            If available, the longitude of the destination.
        latitude:
            If available, the latitude of the destination.
        id:
            The identifier for the task that was performed.
        description:
            Text description provided to this step.
        """
>       array = numpy.asarray(self._routes_numpy())
E       RuntimeError: bad optional access

src/vroom/solution/solution.py:62: RuntimeError
________________________________________________________________________________________ test_repr ________________________________________________________________________________________

    def test_repr():

>       assert (repr(vroom.Vehicle(1, start=4, profile="bus"))
                == "vroom.Vehicle(1, start=4, profile='bus')")
E       assert 'vroom.Vehicl...036854775807)' == "vroom.Vehicl...rofile='bus')"
E         
E         - vroom.Vehicle(1, start=4, profile='bus')
E         + vroom.Vehicle(1, start=4, profile='bus', max_distance=9223372036854775807)

test/test_vehicle.py:6: AssertionError
_____________________________________________________________________________ [doctest] vroom.vehicle.Vehicle _____________________________________________________________________________
089         speed_factor:
090             The speed factor for which this vehicle runs faster or slower than
091             the default.
092         max_tasks:
093             The maximum number of tasks this vehicle can perform.
094         steps:
095             Set of custom steps this vehicle should take.
096 
097     Examples:
098         >>> vroom.Vehicle(1, end=1)
Expected:
    vroom.Vehicle(1, end=1)
Got:
    vroom.Vehicle(1, end=1, max_distance=9223372036854775807)

/pyvroom/src/vroom/vehicle.py:98: DocTestFailure
================================================================================= short test summary info =================================================================================
FAILED README.rst::README.rst
FAILED test/test_amount.py::test_amount_operator - AttributeError: 'Amount' object has no attribute '_lshift'. Did you mean: '__lshift__'?
FAILED test/test_libvroom_examples.py::test_example_with_custom_matrix - RuntimeError: bad optional access
FAILED test/test_vehicle.py::test_repr - assert 'vroom.Vehicl...036854775807)' == "vroom.Vehicl...rofile='bus')"
FAILED src/vroom/vehicle.py::vroom.vehicle.Vehicle
============================================================================== 5 failed, 39 passed in 4.00s ===============================================================================
make: *** [Makefile:14: test] Error 1
SebMilardo commented 4 months ago

Tests are now ok. I had to slightly change the result reported in the README file as sol.summary.duration is now 2702 instead of 2704. I might need to add some tests to check the new features included in 1.14.0.

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 88.05970% with 8 lines in your changes missing coverage. Please review.

Project coverage is 79.1%. Comparing base (da2c48d) to head (4449941).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #106 +/- ## ======================================= - Coverage 80.0% 79.1% -1.0% ======================================= Files 29 29 Lines 1572 1628 +56 Branches 114 139 +25 ======================================= + Hits 1259 1288 +29 - Misses 304 330 +26 - Partials 9 10 +1 ``` | [Flag](https://app.codecov.io/gh/VROOM-Project/pyvroom/pull/106/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VROOM-Project) | Coverage Δ | | |---|---|---| | [binding](https://app.codecov.io/gh/VROOM-Project/pyvroom/pull/106/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VROOM-Project) | `69.1% <87.2%> (-1.7%)` | :arrow_down: | | [python](https://app.codecov.io/gh/VROOM-Project/pyvroom/pull/106/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VROOM-Project) | `91.4% <90.0%> (+0.2%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VROOM-Project#carryforward-flags-in-the-pull-request-comment) to find out more.
jonathf commented 4 months ago

Thanks so much for doing this! This is much appriciated.

I took a quick look, and I want to test a few things before merging. I am also noticing that macos isn't building on release. I am investigating it now.

jonathf commented 4 months ago

I appologies that this has been just sitting there.

As you can see macos isn't building correctly, preventing the build to complete. It is missing an external dep which until now was installed through brew. I can't recreate the bug locally, so the solving this is just a matter of tinkering with deps, paths and/or includes in the CI until I get it right.

But until that issue is fixed, merging this will be moot, as we can not release it.

SebMilardo commented 4 months ago

I also have a mac, I'll try to find a solution

jonathf commented 4 months ago

That is above and beyond! Let me know if you need anything from our side.

SebMilardo commented 4 months ago

I found two major issues:

1) the compiler could not find asio so I had to add it to LDFLAGS and CPPFLAGS 2) jthread is not supported by the included llvm version so I had to update llvm to version 18 and add the "-fexperimental-library'' flag

jcoupey commented 4 months ago

FYI upstream related ticket https://github.com/VROOM-Project/vroom/issues/1062

jonathf commented 3 months ago

Main now has fixes that resolves the issue of building on macos. Hopefully now it should be much easier to get to a 1.14 release.

Let me know if you have the time to do the last stretch, or if you want me to take over.

SebMilardo commented 3 months ago

I merged the changes It should be ok now

jonathf commented 3 months ago

Getting there.

Do you mind moving Homebrew installs into pyproject.toml, flags into setup.py and constant evironment variable under env: directly in the build section?

SebMilardo commented 3 months ago

Sure, no problem

SebMilardo commented 3 months ago

I am trying to solve the issue on macos-intel. As far as I have understood:

the build fails because the minimum target for some libraries is OSX 13.6

  delocate.libsana.DelocationError: Library dependencies do not satisfy target MacOS version 13:
  /private/var/folders/qm/cb4xw0f50z50qsp98fg8rfqm0000gn/T/tmpd7jh26nx/wheel/vroom/.dylibs/libc++.1.0.dylib has a minimum target of 13.6
  /private/var/folders/qm/cb4xw0f50z50qsp98fg8rfqm0000gn/T/tmpd7jh26nx/wheel/vroom/.dylibs/libc++abi.1.0.dylib has a minimum target of 13.6
  /private/var/folders/qm/cb4xw0f50z50qsp98fg8rfqm0000gn/T/tmpd7jh26nx/wheel/vroom/.dylibs/libunwind.1.0.dylib has a minimum target of 13.6

But if I increase MACOSX_DEPLOYMENT_TARGET to 13.6 it fails during the "Testing wheel..." step because 

ERROR: pyvroom-0.1.dev1+g2493307-cp39-cp39-macosx_13_6_x86_64.whl is not a supported wheel on this platform.

as the image is based on macos-13. I also tried adding extra_compile_args.append("-mmacosx-version-min=13.0") but I still get the same "...has a minimum target of 13.6" error.

If I add CIBW_ARCHS_MACOS: x86_64 arm64 to macos-arm to use macos-14 and cross compile for x86_64 I get a different error: ImportError: dlopen(/private/var/folders/sx/mf6nj_sn1ll2crpml02qjgbr0000gn/T/cibw-run-xxe41lvf/cp39-macosx_x86_64/venv-test-x86_64/lib/python3.9/site-packages/vroom/_vroom.cpython-39-darwin.so, 0x0002): symbol not found in flat namespace '_BIO_ctrl'

Any idea? We could:

jonathf commented 3 months ago

Lets try the easy solution and update deploy target to 13.6.

jonathf commented 3 months ago

I just tried updating th target to 13.6 (and then to 14) and made a release(s), but I get the same error unfortunatly.

I prefer not reverting as it will prevent pyvroom from having feature parity with upstream vroom. So for now I think further investigation is warrented.

Am I to understand that you believe jthreads is the likely culprit here?

SebMilardo commented 3 months ago

Using gcc-13 instead of clang solves all the issues. Could you run the workflow now?

jonathf commented 3 months ago

That it does!

Looking through the changes I see that you have not only updated to 1.14, but also fixed a couple of bugs along the way. Excellent work Sebastiano!

SebMilardo commented 3 months ago

The main branch is still failing to build as I forgot to update the main_push.yml file. It should be a quick fix.