ai4co / rl4co

A PyTorch library for all things Reinforcement Learning (RL) for Combinatorial Optimization (CO)
https://rl4.co
MIT License
451 stars 83 forks source link

[BUG] Minimalistic Example #197

Closed leolyg closed 5 months ago

leolyg commented 5 months ago

Hi

The Minimalistic Example is wrong because of "from rl4co.envs.routing import TSPEnv, TSPGenerator". It seems that the TSPGenerator has been removed.

Thank you

Furffico commented 5 months ago

Maybe you are using an earlier version of RL4CO. TSPGenerator was added at v0.4.0, the latest release at the moment. You might get the version number of RL4CO in your env with

python3 -c "import rl4co;print(rl4co.__version__)"

If it's earlier than v0.4.0, here's the minimalistic example for v0.3.3, which should work in earlier versions. Another option is to update the library to the newest version, which is recommended as RL4CO was refactored in v0.4.0.

leolyg commented 5 months ago

Thank you for your reply.

My version is v0.4.0

I mean, in the README file (https://github.com/ai4co/rl4co), the Minimalistic Example code is wrong. TSPGenerator cannot be found there.

from rl4co.envs.routing import TSPEnv, TSPGenerator
from rl4co.models import AttentionModelPolicy, POMO
from rl4co.utils import RL4COTrainer

# Instantiate generator and environment
generator = TSPGenerator(num_loc=50, loc_distribution="uniform")
env = TSPEnv(generator)

# Create policy and RL model
policy = AttentionModelPolicy(env_name=env.name, num_encoder_layers=6)
model = POMO(env, policy, batch_size=64, optimizer_kwargs={"lr": 1e-4})

# Instantiate Trainer and fit
trainer = RL4COTrainer(max_epochs=10, accelerator="gpu", precision="16-mixed")
trainer.fit(model)

The correct one is:

env = TSPEnv(generator_params={'num_loc': 50})

Furffico commented 5 months ago

Oh, I see. Thanks for spotting the issue. Let me test it and open a PR.

Furffico commented 5 months ago

I have tested the code and here's the conclusion: The minimalistic example in the main branch is consistent with the code under active development (v0.5.0dev), so I can't say it's incorrect. However, I think it's better to keep it aligned with the latest released version to reduce such confusions for new users. What do you think, @fedebotu?

fedebotu commented 5 months ago

Thanks for taking care of this issue @Furffico!

I won't have access to a computer these days, but I agree with you, the current README should be consistent with the latest released version. Given we should release the 0.5.0 soon (i.e.: next week), I guess we can provide @leolyg either a working code for the previous version, or preferably, asking to install the latest dev version from source with pip install -U git+https://github.com/ai4co/rl4co.git(which will be released soon anyways)

leolyg commented 5 months ago

Thank you very much. Can't wait to see the new version.