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

Integrate problem changes from PR draft #173 #176

Merged
merged 2 commits into from
Jun 18, 2023
Merged

Conversation

Saethox
Copy link
Collaborator

@Saethox Saethox commented Jun 14, 2023

It was a bit trickier than I expected to entangle the problem definitions from the new component system in #173, but it does compile.

This is the first PR of 3-4 PRs to integrate #173 fully.
It only focuses on the problems, utils and population modules, as the components, framework, conditions, and heuristics, and state modules will be part of the next PR.

Changes

  • Remove old problems in accordance with Move problems::coco and problems::coco_bound into separate crates. #172, Load tsplib using the tspf crate. #169, Extract problems into own repos #155
  • Remove build.rs and gitlab-ci.yml, as not needed anymore
  • Update Cargo.toml (the yet unused packages will be used in the next PRs)
  • Update README with tech report
  • The ObjectiveFunction trait is a simpler version of Evaluator for problems where the objective function can be easily specified, and Sequential<P> and Parallel<P> offer a default implementation of a sequential and parallel Evaluator, respectively, when P implements ObjectiveFunction
  • Simplification and refactoring of the additional problem traits, now with a proper hierarchy (e.g. LimitedVectorProblem depends on VectorProblem, etc.) and a separate TSP trait to access distance information
  • Added TryDefault for the future
  • Added much more extensive documentation to the new modules

Postponed Changes

The following changes had to be postponed because they are too cumbersome to integrate without the new component system:

  • The Evaluator component (now PopulationEvaluator) was changed from accessing the EvaluatorInstance in the State to having a T: Evaluator type parameter
  • This makes it possible to have multiple Evaluators in the state at the same time, and decouples Evaluator and Problem somewhat
  • Problem::default_evaluator was removed in favor of a TryDefault bound on Evaluate

@Saethox Saethox requested review from luleyleo and HeleNoir June 14, 2023 18:00
@Saethox Saethox changed the title Integrate problem changes from PR draft #137 Integrate problem changes from PR draft #173 Jun 14, 2023
Copy link
Collaborator

@HeleNoir HeleNoir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question: Do we have enough documentation in the MAHF readme and the problem repo readmes that everybody knows how to use it?

Copy link
Collaborator

@luleyleo luleyleo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the overall theme, just got a couple of nits to pick.

src/population.rs Outdated Show resolved Hide resolved
src/population.rs Outdated Show resolved Hide resolved
src/population.rs Show resolved Hide resolved
src/problems/evaluate.rs Show resolved Hide resolved
src/problems/mod.rs Outdated Show resolved Hide resolved
src/problems/mod.rs Outdated Show resolved Hide resolved
src/state/common.rs Show resolved Hide resolved
@Saethox
Copy link
Collaborator Author

Saethox commented Jun 15, 2023

Just one question: Do we have enough documentation in the MAHF readme and the problem repo readmes that everybody knows how to use it?

@HeleNoir There is still room for improvement, I'll look into it in the next PR. For example, #111 would be nice to have.

- Remove OptimumReachedProblem
- Change IntoSingle and IntoSingleRef to return a result
@Saethox
Copy link
Collaborator Author

Saethox commented Jun 16, 2023

@luleyleo Are we ready to merge, then?

Copy link
Collaborator

@luleyleo luleyleo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let's merge this!

@Saethox Saethox merged commit e7a520a into master Jun 18, 2023
@Saethox Saethox deleted the update-problems branch June 18, 2023 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants