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

Sweep Visualizations #245

Merged
merged 25 commits into from
May 19, 2023
Merged

Sweep Visualizations #245

merged 25 commits into from
May 19, 2023

Conversation

derpyplops
Copy link
Collaborator

@derpyplops derpyplops commented May 14, 2023

Current usage
elk plot plots your most recent sweep
elk plot --sweep [sweep_name] to visualize particular sweep
elk sweep ... --visualize visualizes after sweeping

Future PRs
elk plot --sweep [sweep_1 .. sweep_n] plots other sweeps
data validation per plot
tests
elk plot --elicit

@CLAassistant
Copy link

CLAassistant commented May 14, 2023

CLA assistant check
All committers have signed the CLA.

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci
@lauritowal lauritowal self-requested a review May 15, 2023 08:38
elk/plotting/visualize.py Outdated Show resolved Hide resolved
elk/training/sweep.py Outdated Show resolved Hide resolved
@derpyplops derpyplops marked this pull request as ready for review May 16, 2023 19:48
elk/plotting/command.py Outdated Show resolved Hide resolved
elk/plotting/utils.py Outdated Show resolved Hide resolved
elk/plotting/visualize.py Outdated Show resolved Hide resolved
elk/plotting/visualize.py Outdated Show resolved Hide resolved
elk/plotting/visualize.py Outdated Show resolved Hide resolved
elk/plotting/visualize.py Outdated Show resolved Hide resolved
elk/plotting/visualize.py Outdated Show resolved Hide resolved
elk/plotting/visualize.py Show resolved Hide resolved
elk/plotting/visualize.py Outdated Show resolved Hide resolved
elk/plotting/visualize.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@lauritowal lauritowal left a comment

Choose a reason for hiding this comment

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

Mostly small things to change

Copy link
Collaborator

@lauritowal lauritowal left a comment

Choose a reason for hiding this comment

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

Please add the commands also to the readme with examples:

Current usage
elk plot plots your most recent sweep
elk plot --sweep [sweep_name] to visualize particular sweep
elk sweep ... --visualize visualizes after sweeping

Future PRs
elk plot --sweep [sweep_1 .. sweep_n] plots other sweeps
data validation per plot
tests
elk plot --elicit

@derpyplops
Copy link
Collaborator Author

@lauritowal any other comments?

@lauritowal lauritowal requested review from norabelrose and removed request for thejaminator May 19, 2023 09:57
Copy link
Collaborator

@lauritowal lauritowal left a comment

Choose a reason for hiding this comment

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

There are still some small things, which I would personally change, but I guess it's okay for now

elk/plotting/visualize.py Show resolved Hide resolved
@derpyplops derpyplops merged commit e77961e into main May 19, 2023
5 of 6 checks passed
@derpyplops derpyplops deleted the visualizations branch May 19, 2023 12:09
@norabelrose norabelrose restored the visualizations branch May 19, 2023 13:08
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

5 participants