[#221] non-distributed optimization algorithm #296
Conversation
There was a problem hiding this comment.
Is this binary file really supposed to be committed?
There was a problem hiding this comment.
This python module is already being published on PyPI: https://pypi.org/project/hyperon-das-node/.
So, a pip install hyperon-das-node==0.0.1 might work.
| MULTIPLY_STRENGTHS = "multiply_strengths" | ||
|
|
||
|
|
||
| def handle_fitness_function(tag: str) -> callable: |
There was a problem hiding this comment.
please rename to compute_fitness_function() if this function is supposed to be called directly or fitness_function_handle() if the idea is to use it to set a variable and call the function indirectly.
This or just rename it to fitness_function().
| atom = atom_db.get_atom(handle) | ||
| if (custom_attributes := atom.custom_attributes): | ||
| if (strength := custom_attributes.get('strength')): | ||
| strengths_value.append(strength) |
There was a problem hiding this comment.
Why storing strength values in a list? Why not just compute the product?
| MAX_CORRELATIONS_WITHOUT_STIMULATE = 1000 | ||
|
|
||
|
|
||
| class SharedQueue: |
There was a problem hiding this comment.
I didn't understand why you are wrapping a Python queue inside this new class. What does this class add?
There was a problem hiding this comment.
It seems you created this wrapper just to have a ShareQueue with the same interface/signatures as the C++ implementation. Any reason for that?
Another question is why there is another quite similar implementation in this PR: https://github.com/singnet/das/blob/masc/221-non-distributed-optimization-algorithm/src/evolution/das_node/shared_queue.py
| """ | ||
| if method == SelectionMethodType.BEST_FITNESS: | ||
| return best_fitness | ||
| elif method == SelectionMethodType.FITNESS_PROPORTIONATE: |
There was a problem hiding this comment.
Do you mean "FITNESS_PROPORTIONAL"? Anyway, even "proportional" is an awful name :-P
Please use ROULETTE instead, which is a more usual name for this function. However, I have two comments which don't need to be addressed in this PR but need to be fixed (please create separate cards for them).
Firstly, this roulette selection method allows the same individual to be selected more than once. Although this is OK in some implementations of GA/GP, in the case of our optimization algorithm tit doesn't seem to make sense IMO.
Secondly, we should implement a tournament selection as well. It's supposed to give equivalent results (when compared to roulette selection) but it's a lot cheaper.
There was a problem hiding this comment.
I think cycle from itertools can be useful here: https://docs.python.org/3/library/itertools.html#itertools.cycle
There was a problem hiding this comment.
Also, this file could follow the same pattern I've suggested for FitnessFunctions.
| COPY asset/hyperon_das_node-0.0.1-cp310-abi3-manylinux_2_28_x86_64.whl /app/ | ||
| RUN pip3 install /app/hyperon_das_node-0.0.1-cp310-abi3-manylinux_2_28_x86_64.whl |
There was a problem hiding this comment.
| pyinstaller --onefile ./evolution/main.py | ||
| mkdir -p bin | ||
| mv ./dist/main ./dist/evolution | ||
| mv ./dist/evolution ./bin/evolution | ||
| rm -rf build/ __pycache__ |
There was a problem hiding this comment.
Is this creating a wheel file?
Please check how the other modules are created using a bazel target:
Lines 35 to 58 in ed760b3
| class FitnessFunctionsTAG(str, enum.Enum): | ||
| MULTIPLY_STRENGTHS = "multiply_strengths" | ||
|
|
||
|
|
||
| def handle_fitness_function(tag: str) -> callable: | ||
| if tag == FitnessFunctionsTAG.MULTIPLY_STRENGTHS: | ||
| return multiply_strengths | ||
| else: | ||
| raise ValueError("Unknown fitness function TAG") | ||
|
|
||
|
|
||
| def multiply_strengths(atom_db, query) -> float: | ||
| strengths_value = [] | ||
| for handle in query.handles: | ||
| atom = atom_db.get_atom(handle) | ||
| if (custom_attributes := atom.custom_attributes): | ||
| if (strength := custom_attributes.get('strength')): | ||
| strengths_value.append(strength) | ||
| # if (truth_value := custom_attributes.get('truth_value')): | ||
| # strength, confidence = truth_value | ||
| # strengths_value.append(strength) | ||
| return math.prod(strengths_value) |
There was a problem hiding this comment.
I think this is cleaner/simpler and also addresses @andre-senna's comment.
Unless I'm not seeing other problems/issues this change could cause.
import math
from typing import ClassVar
def multiply_strengths(atom_db, query) -> float:
strengths_value = []
for handle in query.handles:
atom = atom_db.get_atom(handle)
if (custom_attributes := atom.custom_attributes):
if (strength := custom_attributes.get('strength')):
strengths_value.append(strength)
# if (truth_value := custom_attributes.get('truth_value')):
# strength, confidence = truth_value
# strengths_value.append(strength)
return math.prod(strengths_value)
class FitnessFunctions:
_fitness_functions: ClassVar[dict[str, callable]] = {
"multiply_strengths": multiply_strengths
}
@classmethod
def get(cls, fitness_function_name: str) -> callable:
if fitness_function_name not in cls._fitness_functions:
raise ValueError("Unknown fitness function")
return cls._fitness_functions[fitness_function_name]In the places where it is used (just an example):
from fitness_functions import FitnessFunctions
...
class QueryOptimizerIterator:
...
def _evaluate(self, query_answer: QueryAnswer) -> tuple[QueryAnswer, float]:
""" ... """
fitness_function = FitnessFunctions.get(self.fitness_function)
...
No description provided.