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

API and Code Refactor #107

Closed
BrunoRosendo opened this issue Aug 19, 2024 · 3 comments · Fixed by #108
Closed

API and Code Refactor #107

BrunoRosendo opened this issue Aug 19, 2024 · 3 comments · Fixed by #108

Comments

@BrunoRosendo
Copy link
Owner

The issues with the current code:

  • I used to like the idea of passing all VRP parameters to the solver to simplify the life of the users. However, I now realize this process is not clear at all. It's not natural to copy all the params when simply changing the solver and it's very strange to talk about trips or use_rpp when using a simple VRP model.
  • As development went on, VRP params are repeated all over the place (model, solver, solution). This resulted in cases where I would update a param and the older version would reside in other objects. For this reason, and to reduce code, I suggest always passing and using all information within the model object.

For this, I suggest the following refactor:

  • Add the models to the API, requiring the users to define it before passing it to the solver. This object will be used throughout the code and doesn't need to be modified in order to change the solver.
  • The base models need a refactor, as they contain internal parameters and are used in a different way in Google OR-Tools.
  • I currenty have 5 distinct models (e.g. ConstantCVRP and MultiCVRP), whose names and differences are mere details for end users. I suggest abstracting this to 2 models: CVRP and RPP. They can be simple wrappers which will return the previous models.
@BrunoRosendo
Copy link
Owner Author

After this change, I can finally think about publishing this to PIP

@BrunoRosendo
Copy link
Owner Author

BrunoRosendo commented Aug 19, 2024

the data input also needs to be changed, the scripts, and the API functions need to be created, README

@BrunoRosendo BrunoRosendo linked a pull request Aug 24, 2024 that will close this issue
@BrunoRosendo
Copy link
Owner Author

Now I just need to update the readme and publish the package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant