rng implementation - #200
Conversation
…tils.py and called it where necessary to harmonize the handling of random seeds.
| logger = logging.getLogger("WRT.genetic") | ||
|
|
||
|
|
||
| def get_rng(config: Config, seed=1): |
There was a problem hiding this comment.
Please add a type hint for the return value (numpy.random.Generator) and a short docstring.
|
@kabarmaz Please also update the tests accordingly, particularly test_genetic.py. |
| logger = logging.getLogger("WRT.genetic") | ||
|
|
||
|
|
||
| def get_rng(config: Config, seed=1): |
There was a problem hiding this comment.
Use DEFAULT_RANDOM_SEED as default function argument instead of 1.
| if self.config.GENETIC_FIX_RANDOM_SEED: | ||
| logger.info('Fixing random seed for genetic algorithm.') | ||
| np.random.seed(1) | ||
| seed = 1 |
There was a problem hiding this comment.
Use DEFAULT_RANDOM_SEED instead of 1 (see comments in utils.py).
| super().__init__(config) | ||
|
|
||
| self.config = config | ||
| self.rng = utils.get_rng(config) |
There was a problem hiding this comment.
Remove this line as self.rng is never used in class Genetic.
| constraints_list: ConstraintsList, | ||
| prob=.5, | ||
| crossover_type="unnamed" | ||
|
|
| # ~99.7 % in interval (0, BOAT_SPEED_MAX) | ||
| # self.mu = 0.5 * self.config.BOAT_SPEED_BOUNDARIES[1] | ||
| self.sigma = 1.54 | ||
| self.max_acceleration = 1 |
There was a problem hiding this comment.
Why change these lines? If there is no reason, revert it (see also lines 588 and 589 below).
| shuffled_cost = rng.permutation(shuffled_cost, axis=0) | ||
| shuffled_cost = rng.permutation(shuffled_cost, axis=1) | ||
| shuffled_cost = genetic_utils.RNG.permutation(shuffled_cost, axis=0) | ||
| shuffled_cost = genetic_utils.RNG.permutation(shuffled_cost, axis=1) |
There was a problem hiding this comment.
These changes are not working with the latest updates/current implementation. Please correct.
| self.time_min_max = [None, None] | ||
| self.lat_min_max = [None, None] | ||
| self.lon_min_max = [None, None] | ||
| self.counter = 0 |
| if depth: | ||
| ship_var = ship_var.sel(depth=depth, method='nearest', drop=False) | ||
| ship_var = ship_var.fillna(0).to_numpy() | ||
| self.counter += 1 |
| min_fuel_route_postprocessed.write_to_geojson( | ||
| routepath / f"{min_fuel_route_postprocessed.route_type}_postprocessed.geojson") | ||
|
|
||
| print(boat.counter) |
| default_map = Map(32., 15, 36, 29) | ||
| constraint_list = basic_test_func.generate_dummy_constraint_list() | ||
| np.random.seed(1) | ||
| np.random.default_rng(config.GENETIC_RANDOM_SEED) |
There was a problem hiding this comment.
Remove the line np.random.default_rng(config.GENETIC_RANDOM_SEED).
This returns a generator which is never used.
In the new structure, it's enough to pass config to RandomPlateauMutation because it creates its own random number generator using the seed specified in the config.
| constraint_list = basic_test_func.generate_dummy_constraint_list() | ||
|
|
||
| np.random.seed(1) | ||
| np.random.default_rng(config.GENETIC_RANDOM_SEED) |
There was a problem hiding this comment.
Remove the line np.random.default_rng(config.GENETIC_RANDOM_SEED).
This returns a generator which is never used.
In the new structure, it's enough to pass config to RandomPlateauMutation because it creates its own random number generator using the seed specified in the config.
| default_map = Map(32., 15, 36, 29) | ||
| constraint_list = basic_test_func.generate_dummy_constraint_list() | ||
| np.random.seed(2) | ||
| np.random.default_rng(config.GENETIC_RANDOM_SEED) |
There was a problem hiding this comment.
Remove the line np.random.default_rng(config.GENETIC_RANDOM_SEED).
This returns a generator which is never used.
In the new structure, it's enough to pass config to RouteBlendMutation because it creates its own random number generator using the seed specified in the config.
| constraint_list = basic_test_func.generate_dummy_constraint_list() | ||
|
|
||
| np.random.seed(1) | ||
| np.random.default_rng(config.GENETIC_RANDOM_SEED) |
There was a problem hiding this comment.
Remove the line np.random.default_rng(config.GENETIC_RANDOM_SEED).
This returns a generator which is never used.
In the new structure, it's enough to pass config to RouteBlendMutation because it creates its own random number generator using the seed specified in the config.
| default_map = Map(32., 15, 36, 29) | ||
| constraint_list = basic_test_func.generate_dummy_constraint_list() | ||
| np.random.seed(2) | ||
| np.random.default_rng(config.GENETIC_RANDOM_SEED) |
There was a problem hiding this comment.
Remove the line np.random.default_rng(config.GENETIC_RANDOM_SEED).
This returns a generator which is never used.
Why was np.random.seed(2) used at all? Double-check if the seed was/is considered in the subsequent code.
| departure_time = datetime(2025, 4, 1, 11, 11) | ||
|
|
||
| np.random.seed(2) | ||
| np.random.default_rng(config.GENETIC_RANDOM_SEED) |
There was a problem hiding this comment.
Remove the line np.random.default_rng(config.GENETIC_RANDOM_SEED).
This returns a generator which is never used.
In the new structure, it's enough to pass config to SinglePointCrossover because it creates its own random number generator using the seed specified in the config.
| # FIXME: these numbers should be carefully evaluated | ||
| # ~99.7 % in interval (0, BOAT_SPEED_MAX) | ||
| # self.mu = 0.5 * self.config.BOAT_SPEED_BOUNDARIES[1] | ||
| self.mu = 0.5 * self.config.BOAT_SPEED_BOUNDARIES[1] |
There was a problem hiding this comment.
Re-apply comment as the variable is currently not used.
|
|
||
| self.speed_opts = speed_opts | ||
| self.waypoint_opts = waypoint_opts | ||
| self.config = config |
There was a problem hiding this comment.
Remove self.config = config as it is never used.
| shuffled_cost = rng.permutation(shuffled_cost, axis=0) | ||
| shuffled_cost = rng.permutation(shuffled_cost, axis=1) | ||
| shuffled_cost = self.rng.permutation(shuffled_cost, axis=0) | ||
| shuffled_cost = self.rng.permutation(shuffled_cost, axis=1) |
There was a problem hiding this comment.
self doesn't have an attribute rng.
…tils.py and called it where necessary to harmonize the handling of random seeds.
Related Issue / Discussion:
Relates to issue [https://github.com/52North/WRT-Issues/issues/52]
Changes:
*access counter for weather data
Further Details:
Summary:
Initialized a random number generator in genetic/utils.py to harmonize the random seed and random number generation throughout the genetic algorithm. Implemented into the above listed files.
Dependencies:
Dependencies remain unchanged.
PR Checklist:
In the context of this PR, I:
Please consider that PRs which do not meet the requirements specified in the checklist will not be evaluated. Also, PRs with no activities will be closed after a reasonable amount of time.