Add selection of random seed for reproducibility #39
Conversation
Flaminietta
added
enhancement
|
@kapil-agnihotri, do you have any concerns here or can I merge it? |
mcda/utils/utils_for_main.py
Outdated
| @@ -47,36 +47,41 @@ def check_config_error(condition: bool, error_message: str): | |||
|
|
|||
| def check_config_setting(condition_robustness_on_weights: bool, | |||
| condition_robustness_on_indicators: bool, | |||
| mc_runs: int) -> (int, int): | |||
| mc_runs: int, random_seed: int) -> (int, int): | |||
There was a problem hiding this comment.
Can you try to wrap the method definition in two lines ,i.e., extending the lines upto 120 characters?
I think that will improve the readability. As below we can see lines that are extended until 120.
def check_config_setting(condition_robustness_on_weights: bool, condition_robustness_on_indicators: bool,
mc_runs: int, random_seed: int) -> (int, int):
mcda/utils/utils_for_main.py
Outdated
|
|
||
| Parameters: | ||
| - condition (bool): The condition to check. | ||
| Parameters: |
There was a problem hiding this comment.
List of parameters do not match to that of declared in the method definition.
I think the parameters added here are the expected return parameters and not the ones passed to it. Can you please write both the ones passed and the ones returned in Returns ?
There was a problem hiding this comment.
I have corrected "Parameters" into "Returns". However, I am not adding "Parameters" because my thinking throughout the code has been to add a description of the parameters and/or returns only when I feel it needs a short explanation. Anyways, the last section in every docstring always lists all :param: and :return:.
mcda/utils/utils_for_main.py
Outdated
| "True", "False", 1000) | ||
| ``` | ||
| """ | ||
| :rtype: Tuple[int, int] |
There was a problem hiding this comment.
Here this list is again confusing with the parameter declaration above.
There was a problem hiding this comment.
Do you understand my logic now? Don't you like it?
If needed, a short comment is added about parameters and/or returns, and a full list of parameters and returns is always at the bottom of each docstring.
This produces a function description like this (if you hoover the mouse over its name):
So, it's the bottom list that gives precise info on parameters and returns.
There was a problem hiding this comment.
Cool. Thanks for the explanation.
|
@kapil-agnihotri ready. |
| # TODO: the default_random_seed cannot be used while reading the settings from a JSON file, where None | ||
| # cannot be given as an option; it will be implemented when the congiguration settings will be passed as a | ||
| # stream or handle. | ||
| default_random_seed = 42 |
There was a problem hiding this comment.
I think you don't need an additional variable declaration for 42 I think it can directly be assigned as well. But however, you prefer.
In the configuration file, the user can now select the random seed used for the creation of N random input matrices in case of robustness analysis. This ensures the reproducibility of the results and the possibility of generating different random sequences if needed.
In the function
create_n_randomly_sampled_matricesit is provided thatrandom_seedmay also beNone. This is not possible in the configuration JSON file now but is a generalization we can use when we delete the configuration file.