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

feat: use external training state #18

Merged
merged 15 commits into from
Aug 1, 2022
Merged

feat: use external training state #18

merged 15 commits into from
Aug 1, 2022

Conversation

WissBe
Copy link
Collaborator

@WissBe WissBe commented Jul 27, 2022

No description provided.

@WissBe WissBe requested a review from cyprienc July 27, 2022 10:34
@github-actions
Copy link

github-actions bot commented Jul 27, 2022

Unit Test Results

34 tests   - 1   34 ✔️  - 1   1m 3s ⏱️ +16s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ±0 

Results for commit be24cda. ± Comparison against base commit eeb9bcb.

This pull request removes 9 and adds 8 tests. Note that renamed tests count towards both.
tests.test_catx ‑ test_catx__learn_calls
tests.test_catx ‑ test_catx__sample
tests.test_catx ‑ test_catx__sample_action_range[catx0]
tests.test_catx ‑ test_catx__sample_action_range[catx1]
tests.test_catx ‑ test_catx__sample_action_range[catx2]
tests.test_catx ‑ test_catx__sample_action_range[catx3]
tests.test_catx ‑ test_catx__sample_function_call_count
tests.test_network_builder ‑ test_network_builder
tests.test_network_builder ‑ test_network_builder__create_network
tests.test_catx ‑ test_catx__init_with_extras
tests.test_catx ‑ test_catx__sample[catx_init]
tests.test_catx ‑ test_catx__sample[catx_init_with_extras]
tests.test_catx ‑ test_catx__sample_action_range[catx_init0]
tests.test_catx ‑ test_catx__sample_action_range[catx_init1]
tests.test_catx ‑ test_catx__sample_action_range[catx_init2]
tests.test_catx ‑ test_catx__sample_action_range[catx_init3]
tests.test_network_builder ‑ test_network_module

♻️ This comment has been updated with latest results.

catx/catx.py Show resolved Hide resolved
catx/network_module.py Outdated Show resolved Hide resolved
catx/tree.py Outdated Show resolved Hide resolved
* test: updated convergence catx and contest

* fix: fix mypy type error

* test: updated test_tree

* test: updated network module

* docs: updated getting started guide

* chore: answered review comments
@WissBe WissBe marked this pull request as ready for review August 1, 2022 12:10
catx/network_module.py Outdated Show resolved Hide resolved
docs/getting_started.md Outdated Show resolved Hide resolved
Comment on lines 67 to 82
if not request:
action_min = 0.0
action_max = 1.0
else:
action_min = request.param[0]
action_max = request.param[1]

return mk # type: ignore
catx = CATX(
catx_network=catx_network_with_dropout_extras,
optimizer=optax.adam(learning_rate=0.01),
discretization_parameter=4,
bandwidth=1.5 / 4,
action_min=action_min,
action_max=action_max,
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can probably create a function that takes in the catx_network as parameter as well as the other fixture parameters and returns the CATX object instanciated. Since the fixtures catx and catx_with_dropout_extras differ only by the class used for the catx network.

Copy link
Contributor

@cyprienc cyprienc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - nice work!

@WissBe WissBe merged commit dd9f54a into main Aug 1, 2022
@WissBe WissBe deleted the train-state-switch branch August 3, 2022 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants