-
Notifications
You must be signed in to change notification settings - Fork 30
Rewrite planning
Thoughts on how this should be refactored or rewritten follow.
Wherever the engine
goes, the info_handler
follows. We literally never need the engine without the info_handler too.
A child class to engine
that auto-initializes an info_handler and then passes on other method calls to the parent (maybe using super()
[1], maybe otherwise).
- The functions for manipulating the representation of an evaluation are computationally cheap but litter all over the place, and it's hard to follow which representation is needed in a particular context.
- We're passing lots of arguments around just so we can make sure we always have the information we need to do this representation work.
An evaluation class.
The functions for transforming an evaluation into its various forms can be either method calls or just pre-compute them in the init function. They're so cheap, just do whatever is simpler.
-
Does the evaluation class also create the evaluation on initialization? Or does it accept pre-computed data that it stores?
Probably the former would be more elegant. Creating an engine evaluation is always the same operation so there's no need to reinvent this wheel. Pass in a position, a played move, and an engine, and the thing will do the work on instantiation.
def eval_numeric(info_handler)
def eval_human(white_to_move, info_handler)
def eval_absolute(number, white_to_move)
def winning_chances(centipawns)
Everything I said about evaluations, but applied to judgments.
A judgment class.
Probably combines and synthesizes a bestmove evaluation and playedmove evaluation.
def judge_move()
def get_nags(judgment)
def needs_annotation(judgment)
def var_end_comment(board, judgment)
def add_annotation(node, judgment) # not sure about this one
The classification process is poorly encapsulated, similar to evaluation (just not quite as bad).
TBD
- A new class comes to mind here too, but it's not as obvious that it's really necessary. There's not much state and not much analysis / transformation being done. Maybe they don't have to be bound up like that.
- A functional programming approach might fit here.
- Or just pack the sub-functions into the classification function since they're not used anywhere else.
The way we iterate through moves is unwieldy.
Use the new reversed()
iterator in python-chess 0.24
- Combine that with
itertools.takewhile(predicate, iterable)
to do something like "iterate in reverse until we reach the root node / the classified node". - Honestly there's lots to consider in itertools.
itertools.filterfalse()
could be a better way to give us easy access to only those nodes requiring an annotation.
The way we are calculating ACPL is smelly.
TBD
- Does
functools.reduce()
have applicability for calculating ACPL?