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.

Sign up for GitHub

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

Jump to bottom

Signature of NetworkInterface causing issues in a few files #30

Closed
forrestmckee opened this issue Dec 6, 2022 · 3 comments
Closed

Signature of NetworkInterface causing issues in a few files #30

forrestmckee opened this issue Dec 6, 2022 · 3 comments
Assignees
@ChrisMcCarthyDev
Labels
bug Something isn't working fix_version:1.0.1 Fixed in version 1.0.1

Comments

@forrestmckee
Copy link

Describe the bug
The __init__ method in NetworkInterface changed and no longer allows for settings_path. This causes rl_baseline.py to fail.

To Reproduce
Steps to reproduce the behaviour:

  1. run rl_baseline.py
  2. TypeError

Expected behaviour
I expect the file to run to completion.

Environment (please complete the following information):

  • OS: Ubuntu 18.04
  • Python: 3.8.15
  • YT Version: 1.0.0
  • Software: Google Colab/Notebook

Additional context
This seems like more of a simple oversight than a bug. It looks like there's a new way to do the same thing.
@forrestmckee forrestmckee added the bug Something isn't working label Dec 6, 2022
@jamesshort1
Copy link
Collaborator

Thank you for raising this issue @forrestmckee. We have one of the team looking into it.

@ChrisMcCarthyDev
Copy link
Collaborator

Hi @forrestmckee, thank you for raising this issue!

Firstly let me apologise for this issue, this definitely was an oversight on our part when implementing the updated configs and NetworkInterface instantiation throughout the codebase.

So I have actually identified a number of issues here:

  1. You're right, the NetworkInterface constructor has changed and now takes an instance of GameModeConfig and NetworkConfig. There are three locations in Yawning-Titan that have not had the instantiation of NetworkInterface updated:
    • yawning_titan.integrations.dcbo.rl_baseline
    • yawning_titan.integrations.dcbo.utils
    • utils.custom_config_runner
  2. Yawning-Titan, when installed outside of the cloned repo, has no access to yawning_titan/integrations/dcbo/base_net.txt.
  3. Yawning-Titan, when installed outside of the cloned repo, has no access to yawning_titan/integrations/dcbo/dcbo_config.yaml.
  4. Newly added fields to in the game mode config have not been added to yawning_titan/integrations/dcbo/dcbo_config.yaml.

I'm currently working on a bugfix that will:

  1. Refactor all modules that instantiate the NetworkInterface so that they recieve a GameModeConfig and NetworkConfig.
  2. Convert yawning_titan/integrations/dcbo/base_net.txt to a network function in yawning_titan.envs.generic.helpers.network_creator so the file can be dropped.
  3. Include dcbo_config.yaml as package data and store alongside the default game mode config. Add a reference to its path and update all usages to use this new location.
  4. Add the missing fields to dcbo_config.yaml.

I'm hoping to get a release (v1.0.1) out for this before the end of the week.

@ChrisMcCarthyDev (Yawning-Titan dev team)

@ChrisMcCarthyDev ChrisMcCarthyDev self-assigned this Dec 7, 2022
@ChrisMcCarthyDev ChrisMcCarthyDev added the fix_version:1.0.1 Fixed in version 1.0.1 label Dec 7, 2022
ChrisMcCarthyDev added a commit that referenced this issue Dec 13, 2022
@ChrisMcCarthyDev
Merged PR 7: AIDT-139 - dcbo_config.yaml moved to config package data…
2f89969 
… and path added to ga...

A fix for issue: #30

- dcbo_config.yaml moved to config package data and path added to game_modes.py.
- base_net.txt removed and replaced with dcbo_base_network() in network_creator.py.
- NetworkInterface instantiation fixed in rl_baseline.py, utils.py, and custom_config_runner.py.
- test_dcbo_utils.py updated to test updated ValueError.
- seaborn added to setup.py as it was imported in fig_plotting.py and only just discovered now.
- dependency hell fixed after adding seaborn.
- Docs rebuilt.
- dep_licenses.md regenerated.
- Tests ran.
@ChrisMcCarthyDev
Copy link
Collaborator

Closing as fixed in release v1.0.1.

A-acuto pushed a commit to A-acuto/YAWNING-TITAN that referenced this issue Jan 27, 2023
@ChrisMcCarthyDev
Merge pull request dstl#30 from methodsanalytics/feature/AIDT-99-upda…
90c1316 
…te-documentation-readme-md-and-docs

Feature/aidt 99 update documentation readme md and docs
ChrisMcCarthyDev added a commit that referenced this issue Jun 5, 2023
@ChrisMcCarthyDev
Merge pull request #30 from methodsanalytics/feature/AIDT-99-update-d…
fe9c6e1 
…ocumentation-readme-md-and-docs

Feature/aidt 99 update documentation readme md and docs
ChrisMcCarthyDev added a commit that referenced this issue Jun 5, 2023
@ChrisMcCarthyDev
Merged PR 7: AIDT-139 - dcbo_config.yaml moved to config package data…
111daa1 
… and path added to ga...

A fix for issue: #30

- dcbo_config.yaml moved to config package data and path added to game_modes.py.
- base_net.txt removed and replaced with dcbo_base_network() in network_creator.py.
- NetworkInterface instantiation fixed in rl_baseline.py, utils.py, and custom_config_runner.py.
- test_dcbo_utils.py updated to test updated ValueError.
- seaborn added to setup.py as it was imported in fig_plotting.py and only just discovered now.
- dependency hell fixed after adding seaborn.
- Docs rebuilt.
- dep_licenses.md regenerated.
- Tests ran.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ChrisMcCarthyDev ChrisMcCarthyDev

Labels
bug Something isn't working fix_version:1.0.1 Fixed in version 1.0.1
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ChrisMcCarthyDev @forrestmckee @jamesshort1