Skip to content
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

Remove combined config #485

Merged
merged 63 commits into from
Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
36024c8
solved #484 without check
jannikgro May 26, 2022
473ddf7
introduced JSONConfigurable class and demanded class entry in config
jannikgro May 26, 2022
7cef3e7
added rules and verifications
jannikgro May 26, 2022
b6a4945
reintroduced test_hyperparameter_config_rl
jannikgro May 26, 2022
3fded29
reintroduced test_hyperparameter_config_market
jannikgro May 26, 2022
561fc00
tried to fix config_validation
jannikgro May 26, 2022
6986513
small change in docker_manager
jannikgro May 26, 2022
39bc238
tried with assert False
jannikgro May 26, 2022
75a674e
restored docker_manager
jannikgro May 26, 2022
f1cdf45
debug print
jannikgro May 26, 2022
068abaa
tried with dirty fix
jannikgro May 26, 2022
61c0d8e
tried without reading check
jannikgro May 26, 2022
1d1a3c4
restored last changes
jannikgro May 26, 2022
511d38a
printf debug
jannikgro May 26, 2022
de9f5eb
more printf debug
jannikgro May 26, 2022
09a433e
another printf try
jannikgro May 26, 2022
1e80e8c
next try
jannikgro May 26, 2022
1b139e0
test
jannikgro May 26, 2022
31a212f
more information
jannikgro May 26, 2022
3d3ac9d
more information
jannikgro May 26, 2022
14f181d
restored last changes
jannikgro May 26, 2022
4e174e6
restored changes
jannikgro May 26, 2022
2588f6d
preconditions for actor critic parameters and stable baselines parame…
jannikgro May 26, 2022
4107aaf
Added some debugging to docker
NikkelM May 27, 2022
c90a8d6
Fixed config validation for webserver
NikkelM May 27, 2022
1d025c9
Reintroduced `main` for testing purposes
NikkelM May 27, 2022
dd40104
Removed debug comment, fixed policyanalyzer
NikkelM May 27, 2022
da05474
New feather website
NikkelM May 27, 2022
0fa7eb5
rl config works in view
felix-20 May 30, 2022
26cb54b
Simplified and extended key validation
NikkelM May 31, 2022
7610c6e
script for creating rl model
felix-20 May 31, 2022
16df8c4
Merge branch '484-configuration-remove-combined-config' of https://gi…
felix-20 May 31, 2022
f9aa012
Fixed Agent_monitoring with stable_baselines agents
NikkelM May 31, 2022
c51dab7
renamed rl_config to q_learning_config
NikkelM Jun 1, 2022
2ccee78
Removed `class` keywords from config files
NikkelM Jun 1, 2022
c912309
Removed references to `class`-field
NikkelM Jun 1, 2022
59136b1
Fixed most tests
NikkelM Jun 1, 2022
70492c8
Fixed remaining tests(?)
NikkelM Jun 1, 2022
d326245
fix prefill
felix-20 Jun 1, 2022
135e8f3
fix webserver tests
felix-20 Jun 1, 2022
f50fe65
implement new config validation for webserver
felix-20 Jun 2, 2022
b8c43b7
New config format
NikkelM Jun 2, 2022
caf5c01
Merge branch '484-configuration-remove-combined-config' of https://gi…
NikkelM Jun 2, 2022
fff9b47
Renamed `market` to `sim_market`
NikkelM Jun 2, 2022
dca24f3
Added some tests
NikkelM Jun 2, 2022
c9657a9
Added back needed functionality of validating "complete" configs
NikkelM Jun 2, 2022
b3780cd
Fixed validation
NikkelM Jun 2, 2022
57dbb33
Webserver format
NikkelM Jun 2, 2022
bce03bd
fix webserver tests
felix-20 Jun 2, 2022
507cbc1
dynamic table for sim_market possible
felix-20 Jun 3, 2022
dac92d3
Fixed tests
NikkelM Jun 3, 2022
bd15108
Merge branch '484-configuration-remove-combined-config' of https://gi…
NikkelM Jun 3, 2022
e623413
Added `config_type` field to default modelfiles
NikkelM Jun 3, 2022
179e0da
fix javascript error
felix-20 Jun 3, 2022
6a09267
Merge branch '484-configuration-remove-combined-config' of https://gi…
felix-20 Jun 3, 2022
99ab681
Fixed config validation
NikkelM Jun 4, 2022
53d79f9
Review feedback by @SinNeax
NikkelM Jun 7, 2022
8839f90
remove `config_is_final`
felix-20 Jun 7, 2022
ab704ca
improved prefill (#496)
felix-20 Jun 7, 2022
bc9d3a3
Removed dead methods
NikkelM Jun 7, 2022
af12e13
Merge branch '484-configuration-remove-combined-config' of https://gi…
NikkelM Jun 7, 2022
ae9e85b
added common-rule for number_customers
nick-bessin Jun 8, 2022
10cc747
removed unnecessary else-condition
nick-bessin Jun 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion docker/docker_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ def ping(self) -> bool:
return self._get_client().ping()
except Exception:
print('Docker server is not responding!')
print(f'Client is: {self._get_client()}')
return False

# PRIVATE METHODS
Expand All @@ -320,8 +321,10 @@ def _get_client(cls) -> docker.DockerClient:
if cls._client is not None:
return cls._client
try:
print('Trying to get a new client from docker...')
cls._client = docker.from_env()
except docker.errors.DockerException:
except docker.errors.DockerException as e:
print(f'Got a DockerException: {e}')
cls._client = None
return cls._client

Expand Down
1 change: 1 addition & 0 deletions environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ dependencies:
- coverage>=5.5
- coverage-badge>=1.1.0
- pytest=6.2.4
- lxml>=4.8.0
- pytest-randomly=3.11.0
- pytest-xdist=2.5.0
14 changes: 14 additions & 0 deletions recommerce/configuration/common_rules.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
def greater_zero_rule(field_name: str):
return (lambda x: x > 0, f'{field_name} should be positive')


def non_negative_rule(field_name: str):
return (lambda x: x >= 0, f'{field_name} should be non-negative')


def between_zero_one_rule(field_name: str):
return (lambda x: x >= 0 and x <= 1, f'{field_name} should be between 0 (included) and 1 (included)')


def greater_zero_even_rule(field_name: str):
return (lambda x: x > 0 and x % 2 == 0, f'{field_name} should be even and positive')
191 changes: 70 additions & 121 deletions recommerce/configuration/config_validation.py
Original file line number Diff line number Diff line change
@@ -1,161 +1,108 @@
# This file contains logic used by the webserver to validate configuration files


from recommerce.configuration.environment_config import EnvironmentConfig
from recommerce.configuration.hyperparameter_config import HyperparameterConfigValidator
from recommerce.configuration.utils import get_class


def validate_config(config: dict, config_is_final: bool) -> tuple:
def validate_config(config: dict) -> tuple:
"""
Validates a given config dictionary either uploaded by the user or entered into the form before starting a container.

Args:
config (dict): The config to validate.
config_is_final (bool): Whether or not the config must contain all required keys.

Returns:
tuple: success: A status (True) and the split hyperparameter_config and environment_config dictionaries as a tuple.
failure: A status (False) and the errormessage as a string.
"""
try:
# first check if the environment and hyperparameter parts are already split up
# first check if we got a complete config from the webserver
# in which case we will have two keys on the top-level
# this can either be an uploaded complete config, or a config sent when pressing the launch/check button
if 'environment' in config and 'hyperparameter' in config:
assert len(config) == 2, 'Your config should not contain keys other than "environment" and "hyperparameter"'
hyperparameter_config = config['hyperparameter']
environment_config = config['environment']
elif 'environment' in config or 'hyperparameter' in config:
raise AssertionError('If your config contains one of "environment" or "hyperparameter" it must also contain the other')
else:
# try to split the config. If any keys are unknown, an AssertionError will be thrown
hyperparameter_config, environment_config = split_mixed_config(config)
# then validate that all given values have the correct types
check_config_types(hyperparameter_config, environment_config, config_is_final)

if 'rl' in hyperparameter_config:
HyperparameterConfigValidator.check_rl_ranges(hyperparameter_config['rl'], config_is_final)
if 'sim_market' in hyperparameter_config:
HyperparameterConfigValidator.check_sim_market_ranges(hyperparameter_config['sim_market'], config_is_final)
market_class = get_class(environment_config['marketplace'])
agent_class = get_class(environment_config['agents'][0]['agent_class'])

HyperparameterConfigValidator.validate_config(hyperparameter_config['sim_market'], market_class)
HyperparameterConfigValidator.validate_config(hyperparameter_config['rl'], agent_class)
EnvironmentConfig.check_types(environment_config, environment_config['task'], False, True)

return True, ({'rl': hyperparameter_config['rl']}, {'sim_market': hyperparameter_config['sim_market']},
{'environment': environment_config})
# if the two keys are not present, the config MUST be one of environment, rl, or market
# this is only the case when uploading a config
else:
config_type = find_config_type(config)

# we can only validate types for the environment_config, as we do not know the agent/market class for the rl/market configs
if config_type == 'environment':
# validate that all given values have the correct types
task = config['task'] if 'task' in config else 'None'
EnvironmentConfig.check_types(config, task, False, False)

# the webserver needs another format for the config
config.pop('config_type')
if config_type == 'rl':
return True, ({'rl': config}, None, None)
elif config_type == 'sim_market':
return True, ({'sim_market': config}, None, None)
else:
return True, ({'environment': config}, None, None)

return True, (hyperparameter_config, environment_config)
except Exception as error:
return False, str(error)


def validate_sub_keys(config_class: HyperparameterConfigValidator or EnvironmentConfig, config: dict, top_level_keys: dict) -> None:
def find_config_type(config: dict) -> str:
"""
Utility function that validates if a given config contains only allowed keys.
Can be used recursively for dictionaries within dictionaries.
"Unisex": Works for both HyperparameterConfigValidator and EnvironmentConfig.
Extract the config type from the config dictionary. Config type is defined by the "config_type" key, which must always be present.

Args:
config_class (HyperparameterConfigValidator or EnvironmentConfig): The config class from which to get the required fields.
config (dict): The config given by the user.
top_level_keys (dict): The keys of the current level. Their values indicate if there is another dictionary expected for that key.
config (dict): The config to check.

Raises:
AssertionError: If the given config contains a key that is invalid.
"""
for key, _ in config.items():
# we need to separately check agents, since it is a list of dictionaries
if key == 'agents':
assert isinstance(config['agents'], list), f'The "agents" key must have a value of type list, but was {type(config["agents"])}'
for agent in config['agents']:
assert isinstance(agent, dict), f'All agents must be of type dict, but this one was {type(agent)}'
assert all(agent_key in {'name', 'agent_class', 'argument'} for agent_key in agent.keys()), \
f'An invalid key for agents was provided: {agent.keys()}'
# the key is key of a dictionary in the config
elif top_level_keys[key]:
assert isinstance(config[key], dict), f'The value of this key must be of type dict: {key}, but was {type(config[key])}'
# these are the valid keys that sub-key can have as keys in the dictionary
key_fields = config_class.get_required_fields(key)
# check that only valid keys were given by the user
for sub_key, _ in config[key].items():
assert sub_key in key_fields.keys(), \
f'The key "{sub_key}" should not exist within a {config_class.__name__} config (was checked at sub-key "{key}")'
# if there is an additional layer of dictionaries, check it recursively
validate_sub_keys(config_class, config[key], key_fields)


def split_mixed_config(config: dict) -> tuple:
"""
Utility function that splits a potentially mixed config of hyperparameters and environment-variables
into two dictionaries for the two configurations.

Args:
config (dict): The potentially mixed configuration.
AssertionError: If the config_type key has an invalid value or is missing.

Returns:
dict: The hyperparameter_config
dict: The environment_config

Raises:
AssertionError: If the user provides a key that should not exist.
str: The config type.
"""
top_level_hyperparameter = HyperparameterConfigValidator.get_required_fields('top-dict')
top_level_environment = EnvironmentConfig.get_required_fields('top-dict')

hyperparameter_config = {}
environment_config = {}

for key, value in config.items():
if key in top_level_hyperparameter.keys():
hyperparameter_config[key] = value
elif key in top_level_environment.keys():
environment_config[key] = value
try:
if config['config_type'] in ['rl', 'sim_market', 'environment']:
return config['config_type']
else:
raise AssertionError(f'Your config contains an invalid key: {key}')

validate_sub_keys(HyperparameterConfigValidator, hyperparameter_config, top_level_hyperparameter)
validate_sub_keys(EnvironmentConfig, environment_config, top_level_environment)

return hyperparameter_config, environment_config


def check_config_types(hyperparameter_config: dict, environment_config: dict, must_contain: bool = False) -> None:
"""
Utility function that checks (incomplete) config dictionaries for their correct types.

Args:
hyperparameter_config (dict): The config containing hyperparameter_config-keys.
environment_config (dict): The config containing environment_config-keys.
must_contain (bool): Whether or not the configuration should contain all required keys.

Raises:
AssertionError: If one of the values has the wrong type.
"""
# check types for hyperparameter_config
# @NikkelM Why was this here?
# HyperparameterConfigValidator.check_types(hyperparameter_config, 'top-dict', must_contain)
if 'rl' in hyperparameter_config:
HyperparameterConfigValidator.check_types(hyperparameter_config['rl'], 'rl', must_contain)
if 'sim_market' in hyperparameter_config:
HyperparameterConfigValidator.check_types(hyperparameter_config['sim_market'], 'sim_market', must_contain)

# check types for environment_config
task = environment_config['task'] if must_contain else 'None'
EnvironmentConfig.check_types(environment_config, task, False, must_contain)
raise AssertionError(f'the "config_type" key must be one of "rl", "sim_market", "environment" but was {config["config_type"]}')
except KeyError as e:
raise AssertionError(f"your config is missing the 'config_type' key, must be one of 'rl', 'sim_market', 'environment': {config}") from e


if __name__ == '__main__': # pragma: no cover
test_config = {
'rl': {
'batch_size': 32,
'replay_size': 100000,
'learning_rate': 1e-6,
'sync_target_frames': 1000,
'replay_start_size': 10000,
'epsilon_decay_last_frame': 75000,
'epsilon_start': 1.0,
'epsilon_final': 0.1
},
'sim_market': {
'max_storage': 100,
'episode_length': 50,
'max_price': 10,
'max_quality': 50,
'production_price': 3,
'storage_cost_per_product': 0.1
},
test_config_rl = {
'config_type': 'rl',
'batch_size': 32,
'replay_size': 100000,
'learning_rate': 1e-6,
'sync_target_frames': 1000,
'replay_start_size': 10000,
'epsilon_decay_last_frame': 75000,
'epsilon_start': 1.0,
'epsilon_final': 0.1
}
test_config_market = {
'config_type': 'sim_market',
'max_storage': 100,
'episode_length': 50,
'max_price': 10,
'max_quality': 50,
'production_price': 3,
'storage_cost_per_product': 0.1
}
test_config_environment = {
'config_type': 'environment',
'episodes': 5,
'agents': [
{
Expand All @@ -170,5 +117,7 @@ def check_config_types(hyperparameter_config: dict, environment_config: dict, mu
}
]
}
hyper, env = split_mixed_config(test_config)
check_config_types(hyper, env)
print('Testing config validation...')
print(validate_config(test_config_rl))
print(validate_config(test_config_market))
print(validate_config(test_config_environment))
8 changes: 2 additions & 6 deletions recommerce/configuration/environment_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
from recommerce.market.linear.linear_vendors import LinearAgent
from recommerce.market.sim_market import SimMarket
from recommerce.market.vendors import FixedPriceAgent
from recommerce.rl.actorcritic.actorcritic_agent import ActorCriticAgent
from recommerce.rl.q_learning.q_learning_agent import QLearningAgent
from recommerce.rl.reinforcement_learning_agent import ReinforcementLearningAgent


Expand Down Expand Up @@ -159,7 +157,7 @@ def _parse_and_set_agents(self, agent_list: list, needs_modelfile: bool) -> None
agent['agent_class'] = get_class(agent['agent_class'])

# This if-else contains the parsing logic for the different types of arguments agents can have, e.g. modelfiles or fixed-price-lists
if needs_modelfile and issubclass(agent['agent_class'], (QLearningAgent, ActorCriticAgent)):
if needs_modelfile and issubclass(agent['agent_class'], ReinforcementLearningAgent):
assert isinstance(agent['argument'], str), \
f'The "argument" field of this agent ({agent["name"]}) must be a string but was ({type(agent["argument"])})'
assert agent['argument'].endswith('.dat') or agent['argument'].endswith('.zip'), \
Expand Down Expand Up @@ -291,7 +289,7 @@ def _validate_config(self, config: dict) -> None:
self.agent = []
for current_agent in passed_agents:
# with modelfile
if issubclass(current_agent['agent_class'], (QLearningAgent, ActorCriticAgent)):
if issubclass(current_agent['agent_class'], ReinforcementLearningAgent):
self.agent.append((current_agent['agent_class'], [current_agent['argument'], current_agent['name']]))
# without modelfile
else:
Expand All @@ -312,8 +310,6 @@ class ExampleprinterEnvironmentConfig(EnvironmentConfig):
"""
def _validate_config(self, config: dict) -> None:
super(ExampleprinterEnvironmentConfig, self)._validate_config(config, single_agent=False, needs_modelfile=True)
# Since we only have one agent, we extract it from the provided list
self.agent = self.agent[0]
nick-bessin marked this conversation as resolved.
Show resolved Hide resolved

def _get_task(self) -> str:
return 'exampleprinter'
Expand Down
Loading