search

AlignmentResearch / KataGoVisualizer

MIT License
3 stars 1 forks source link

ICML: New adversarial training plots + adjust plot sizes + make plots prettier #84

Closed ed1d1a8d closed 1 year ago

ed1d1a8d commented 1 year ago
review-notebook-app[bot] commented 1 year ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

netlify[bot] commented 1 year ago

Deploy Preview for goattack canceled.

Name Link
Latest commit 1db6fa11a9811aeba0cbdbe742558189623743df
Latest deploy log https://app.netlify.com/sites/goattack/deploys/64920749a321050008d555d2
review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

tomtseng commented on 2023-06-05T23:18:44Z ----------------------------------------------------------------

Line #16.    from statsmodels.stats.proportion import proportion_confint

nit: utils is a local import while this is a 3rd-party library import, so i think utils should not be grouped with all these 3rd-party imports


_ed1d1a8d commented on 2023-06-20T19:51:04Z_ ----------------------------------------------------------------

Was organizing imports using isort and isort wasn't recognizing utils and sgf_parser as local imports. I've now created a isort.cfg that specifies this so isort auto-formatting does the right thing.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

tomtseng commented on 2023-06-05T23:18:45Z ----------------------------------------------------------------

Line #16.                point on the resulting plot.

Should add description of these args to the header function:

adv_name: Human-readable name of the adversary
legend_args: Additional arguments for the matplotlib legend.

Not sure if legend_args is strictly necessary — could the caller just take axs from the return value and call axs.legend(...) separately themselves?


_ed1d1a8d commented on 2023-06-20T20:07:47Z_ ----------------------------------------------------------------

Added adv_name description and removed legend_args

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

tomtseng commented on 2023-06-05T23:22:37Z ----------------------------------------------------------------

Line #9.    import utils

nit: utils should be grouped on its own line


review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

tomtseng commented on 2023-06-05T23:22:38Z ----------------------------------------------------------------

Line #13.                adv_name="Cylic",

Typo: Cylic -> Cyclic


ed1d1a8d commented 1 year ago

Was organizing imports using isort and isort wasn't recognizing utils and sgf_parser as local imports. I've now created a isort.cfg that specifies this so isort auto-formatting does the right thing.

View entire conversation on ReviewNB

ed1d1a8d commented 1 year ago

Added adv_name description and removed legend_args

View entire conversation on ReviewNB

  • © Githubissues.
  • Githubissues is a development platform for aggregating issues.