torressa / cspy

A collection of algorithms for the (Resource) Constrained Shortest Path problem in Python / C++ / C#
https://torressa.github.io/cspy/
MIT License
77 stars 24 forks source link

PSOLGENT improvements #80

Closed dvbuntu closed 2 years ago

dvbuntu commented 3 years ago
dvbuntu commented 3 years ago

Sorry for all the builds failing. I haven't been able to properly build and test myself. I'll try to sort out the issues here and push one more time.

dvbuntu commented 3 years ago

Not clear to me why the ubuntu Upload docs failed now (didn't need to change anything in docs), but I believe everything is in order.

dvbuntu commented 3 years ago

Hmm, I need to work on the rand value saving a bit more. I didn't realize _get_fitness was called in multiple places. If I don't get to this today, I'll definitely sort it out next week.

torressa commented 3 years ago

Thanks a lot for this! Don't worry about the ubuntu docs build. I need to fix it as it's not doing what it should be.

Once you're ready, just add the changes (list in PR description works) to the changelog and bump to 1.0.1. (in the cmake file here). Once it's merged to master this will upload the new wheels to pypi and we'll be good.

dvbuntu commented 3 years ago

Latest push should close this out (though latest issue #82 calls into questions PSOLGENT's general utility).

sourcery-ai[bot] commented 3 years ago

Sourcery Code Quality Report

❌  Merging this PR will decrease code quality in the affected files by 5.58%.

Quality metrics Before After Change
Complexity 4.26 ⭐ 4.96 ⭐ 0.70 👎
Method Length 55.81 ⭐ 63.93 🙂 8.12 👎
Working memory 9.61 🙂 11.35 😞 1.74 👎
Quality 70.29% 🙂 64.71% 🙂 -5.58% 👎
Other metrics Before After Change
Lines 585 674 89
Changed files Quality Before Quality After Quality Change
src/python/algorithms/grasp.py 74.86% 🙂 70.02% 🙂 -4.84% 👎
src/python/algorithms/path_base.py 67.32% 🙂 66.23% 🙂 -1.09% 👎
src/python/algorithms/psolgent.py 69.42% 🙂 61.34% 🙂 -8.08% 👎

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation
src/python/algorithms/psolgent.py PSOLGENT._update_best 4 ⭐ 240 ⛔ 22 ⛔ 38.47% 😞 Try splitting into smaller methods. Extract out complex expressions
src/python/algorithms/path_base.py PathBase.get_simple_path 14 🙂 158 😞 13 😞 43.66% 😞 Try splitting into smaller methods. Extract out complex expressions
src/python/algorithms/psolgent.py PSOLGENT.__init__ 1 ⭐ 173 😞 28 ⛔ 44.45% 😞 Try splitting into smaller methods. Extract out complex expressions
src/python/algorithms/psolgent.py PSOLGENT.run 10 🙂 191 😞 11 😞 47.50% 😞 Try splitting into smaller methods. Extract out complex expressions
src/python/algorithms/path_base.py PathBase.check_feasibility 12 🙂 156 😞 9 🙂 52.85% 🙂 Try splitting into smaller methods

Legend and Explanation

The emojis denote the absolute quality of the code:

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.


Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

dvbuntu commented 3 years ago

Just pushed another commit. It should address the Issue #82, and in the process, I improved the way GRASP generates local search candidates (i.e. sample from only valid edges rather than a hypothetical fully connected graph).

There's still something funky going on with what should be deterministic processes. I fixed one place in GRASP that was stealthily using its own random, but that didn't seem to banish the issue. I have gotten all tests to pass before, but I wouldn't be surprised if some claimed failure.

After working through this, I'm not sure this approach is effective. It's not going to scale to large graphs where the number of local search possibilities is huge. I don't have time right now, but in the future I still might try the "2nd dimension for node order" idea. I think that would be cleaner and more repeatable.

torressa commented 3 years ago

Nice one, very neat! You are right, this is not really scaleable and the randint is not reliable.

I'll try to implement another approach closer to the paper: try all feasible swaps of nodes in a path except source/sink see if any of these combinations improves the obj. This should be pretty straight forward using the framework you've put down.

So hopefully this weekend we can have a minor release.

By the way, are you building locally now? If so, how did you manage? Whenever you find some time, give it a shot!! It could a very interesting contribution

dvbuntu commented 3 years ago

Thanks for working with me on this PR. I have been building locally in that I got the containerized testing/building to work. I wish I had more time to work on this, but I may be busy until later this year.

torressa commented 2 years ago

Merged in master.