-
Notifications
You must be signed in to change notification settings - Fork 0
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
Prepare MAHF for crates.io #173
Conversation
- Remove built-in problems in favor of separate repos - Split Evaluator further into ObjectiveFunction - Fix the encoding of VectorProblem - Add a separate trait for TSP
- MutState was removed in favor of Ref and RefMut - StateRegistry handles borrowing and state management - State allows access to problem-specific common state - Added entry API - Added separate handle for state requirements - Added error handling to state access
- Add a separate require phase additionally to init - Remove generation module in favor of having separate modules for mutation, recombination, swarm, and generative components - Keep state-modifying components here instead of the state module - Split complicated components functionality into separate functions with own unit tests - Add an add_params() method to all components, which returns Self rather than the boxed trait - Update PopulationEvaluator to use a generic Evaluator - Store more parameters in the state to allow for adaption - Introduce identifiers to distinguish between similar state and components - Remove the concept of termination in favor of simple conditions -
- Rename LogSet into LogConfig - Allow multiple extractors for the same trigger - Remove Trigger in favor of Condition - Integrate saving to multiple file formats into Log - Integrate the Extract API with log entries
- Always insert the Logger - Allow specifying the Evaluator separately
While this sounds really good in general, would it be possible to split this PR in smaller ones, each focussed on a specific aspect? Reviewing everything as a whole is a bit overwhelming and particularly prone to errors. Generally, we need a better PR workflow. If its not (easily) possible, we can leave the PR as is, but next time, smaller ones would be better. |
@Saethox how much work would it be to split this up? Or at least move everything, where it is convenient, into separate PRs. For example, removing the old problem modules could easily be separated, right? |
I know, right 🙈. I was a little much too focused on getting it all to work together.
Splitting the problems is easily done, the other changes are more cumbersome. I'm in no real hurry to merge this (maybe before GECCO), so I could probably split this PR into three/four incremental PRs:
Depending on how we want to handle the problem dependencies ( We can then just use this draft as a proof that all of the concepts do work together, so I guess that's nice to know 🤷♂️. |
I think 3 or 4 PRs would be okay.
Well, I would prefer as soon as possible, but before GECCO would be okay. I have to start implementing #75 and I also need to play around with the logging a bit to get data for the evaluation (so I can finally implement it completely) and I fear if I start before this PR is done, I will cause additional problems. |
That's not really a problem, porting components to the new state management and returning a result instead of panicking is done in a few minutes, It's not like the functionality of the components gets any different through this, especially for specialized operators.
Again, the general concept of logging is not that different, it's just more flexible than before. The log format is also exactly the same. |
Integrate problem changes from PR draft #173
Integrate most of the changes from PR #173
I'm making this a draft for now, so we can discuss the design decisions before anything is merged. This is a PR that emerged from some experimental changes I wanted to try out and have integrated before we publish on crates.io, and it subsequently got somewhat out of control (the line diff scares me).
In general, I'm pretty happy with how it turned out, but there is still much work to do apart from all of this.
Documentation
This PR should include
lib
,components
,conditions
,state
,logging
,heuristics
, again with examplesI just didn't want to spend time documenting stuff we might change, so I put it off until now. It should definitely be part of this PR though.
Rustfmt
I added a
rustfmt.toml
with the options mentioned in #164, and adapted theci.yml
to use nighlyrustfmt
, because these options are nightly.clippy
,check
, andtest
are still performed onstable
just like before. I just can't go back to the old imports after I have seen the beauty of what nightlyrustfmt
is capable of.Changes
Problems
Evaluator
component (nowPopulationEvaluator
) was changed from accessing theEvaluatorInstance
in theState
to having aT: Evaluator
type parameterEvaluator
s in the state at the same time, and decouplesEvaluator
andProblem
somewhatProblem::default_evaluator
was removed in favor of aTryDefault
bound onEvaluator
ObjectiveFunction
trait is a simpler version ofEvaluator
for problems where the objective function can be easily specified, andSequential<P>
andParallel<P>
offer a default implementation of a sequential and parallelEvaluator
, respectively, whenP
implementsObjectiveFunction
LimitedVectorProblem
depends onVectorProblem
, etc.) and a separate TSP trait to access distance informationState
MutState
is no more:State
now allows for proper (mutable) borrowing of custom state at the same time (makingRef
andRefMut
part of the public API is kind of annoying, but it's still so much better than before IMO)HashMap::entry
State<P>
is now only a thin wrapper around theStateRegistry
, to allow accessing custom state without having to addP
as a type parameter everywhere where it's really not necessary (especially relevant for the newExtract
mechanism)Extract
,ExtractRef
, andExtractRefMut
now make it possible to access custom state without caring what the exact wrapper type is&self
) for several reasons: dealing with boxed extractors was a major hassle, they are meant to only map values, not really modify them, and adding them through type parameters allows a pretty readable syntax, imoIdFn<T>
makes it possible to retrieve any custom stateValueOf<T>
makes it possible to retrieve theDeref::Target
of a wrapper type, which is really handyLessThan<ValueOf<Iterations>>::new(10)
instead of implementing separateFixedIterations
,FixedEvaluations
, etc. conditionsLinear<I, O>
component taking an input value from the extractorI
and mapping it linearly onto the custom state extracted byO
State
now definestry_
methods which return aResult<_, StateError>
Components and Conditions
initialize
was renamed toinit
require
phase betweeninit
andexecute
, where only requirements can be definedswarm
andgenerative
modules are now meant to hold PSO and ACO-like components, respectively, if we add moreComponent
andCondition
methods now return aResult
eyre
was used as error library because it has better backtrace information thananyhow
, and a custom error type would be much more hasslefrom_params()
method, which returns the unboxed component/conditionUniformMutation
andNormalMutation
store theirMutationRate
in the state, but if they use the same type they overwrite each otherMutationRate
generic over the type of the component that stores it -->MutationRate<UniformMutation>
Identifier
type -->UniformMutation<I: Identifier = Global>
Identifier
isGlobal
, but can be replaced withA
,B
, etc., or own types<>
if the default identifier is used, but it's not really a problem (should be documented though because it took me a while to figure it out)termination
was removed, there are now only conditionsLessThan
,EveryN
, andChangeOf
conditions are now very flexible in combination with extractorsMutator
,Initializer
, etc. were replaced with functions doing the same thinginit
orrequire
was very awkward for components only implementing the simpler trait, and would require adding these methods to each simpler traitSelector(ComponentImplementingSelection)
, which made an implementation detail part of the serializationSelector
were not easily retraceable to the component that was wrappedComponent
, which is a little bit more code, but has none of these flawsinit
orrequire
, we could add a derive macro that accepts the translator function and simply calls it in theexecute
implementationLogging
tracking
was renamed intologging
Trigger
was removed in favor of just usingCondition
sCondition
s have aSerialize
bound, but having a separateTrigger
either leads to much code duplication or very weird conversion codeTrigger
, it might as well be reused as aCondition
, so it can be one in the first placeExtractor
(nowEntryExtractor
) trait is now built on top of theExtract
API, which again is very comfortable because everything can be reusedLogSet
is nowLogConfig
, because this makes more sense IMOOther
par_experiment
helper was added for dealing with experiments, this could (and probably should) be extended into a whole experiment framework, maybe some sort ofExperiment
builder with a bunch of options