[Feat] add GFACS and replace PyVRP with HGS for CVRP local search #236
Conversation
| @@ -58,7 +58,7 @@ def forward( | |||
|
|
|||
| def pre_decoder_hook( | |||
| self, td: TensorDict, env: RL4COEnvBase, hidden: Any = None, num_starts: int = 0 | |||
| ) -> Tuple[TensorDict, Any, RL4COEnvBase]: | |||
| ) -> Tuple[TensorDict, RL4COEnvBase, Any]: | |||
There was a problem hiding this comment.
About HGS-CVRP: I think the best way for us would be to provide instructions for the user on how to download this repo and install it? If the code was modified (as it seems to be) I think we can upload it as a separate repo that can be downloaded as optional - this is for us not to maintain in a single repo RL-based solvers and pure heuristics 👀
For example, we could provide instructions like this:
cd rl4co/envs/routing/cvrp
git clone [email protected]:vidalt/HGS-CVRP.git # can be modified, e.g. git clone [email protected]:ai4co/HGS-CVRP.git if we upload it as separate code
cd HGS-CVRP/
bash build.sh There was a problem hiding this comment.
Good point. Yes, it's modified by DeepACO.
But I'm wondering if it is worth uploading it as a separate repo because it is almost the same as the original HGS but has a function for local_search.
@henry-yeh, @Furffico Do you guys have any idea?
There was a problem hiding this comment.
Both options seem fine to me, but I lean slightly towards optional downloading. It would make RL4CO more purely RL-based and lighter to use.
| Params params(x_coords,y_coords,distance_matrix,service_time,demands,vehicleCapacity,durationLimit,max_nbVeh,isDurationConstraint,verbose,*ap); | ||
|
|
||
| char buff[100] = {}; | ||
| snprintf(buff, sizeof(buff), "/tmp/route-%i", callid); |
There was a problem hiding this comment.
If Windows compatibility is taken into consideration, some modifications might be necessary here and in local_search.py.
For instance, in Python, using tempfile.TemporaryDirectory() to obtain a cross-platform temporary directory path and then passing it to this function.
However, considering Windows compatibility might bring additional complications (build scripts, documentations, etc.). It's up to you.
| @@ -257,7 +256,7 @@ def replace_selected_actions( | |||
| def local_search(td: TensorDict, actions: torch.Tensor, **kwargs) -> torch.Tensor: | |||
| assert ( | |||
| local_search is not None | |||
| ), "Cannot import local_search module. Check if `pyvrp` is installed." | |||
| ), "Cannot import local_search module. Check if HGS is built." | |||
There was a problem hiding this comment.
Perhaps some instructions required here to tell users on how to build HGS, e.g. "Please check ... for more information."
|
If it's not an issue, I think GFACS can be released in the next This would allow us to release |
|
Yeap sure. I think I can also add my current ICML project. |
This would be awesome 🚀 |
Description
This PR includes:
Regarding documentation and tests: I added/updated docstrings, but not very carefully. I didn't update tests. If you think more work is needed in these areas, please ping me.
Types of changes
What types of changes does your code introduce? Remove all that do not apply:
Checklist
Go over all the following points, and put an
xin all the boxes that apply.If you are unsure about any of these, don't hesitate to ask. We are here to help!