Closed ed1d1a8d closed 1 year ago
View / edit / reply to this conversation on ReviewNB
tomtseng commented on 2023-06-06T04:27:09Z ----------------------------------------------------------------
Line #53. plt.xlabel("Cyclic-adversary finetuning GPU days")
nit: we write "[V100] GPU-days" (hyphen) instead of "GPU days" (no hyphen) elsewhere in this notebook and in the paper — we should be consistent
View / edit / reply to this conversation on ReviewNB
tomtseng commented on 2023-06-06T04:30:12Z ----------------------------------------------------------------
nit: we write "fine-tune" instead of "finetune" in the paper, we should be consistent about which one we write
View / edit / reply to this conversation on ReviewNB
tomtseng commented on 2023-06-09T21:21:12Z ----------------------------------------------------------------
Line #19. continue_steps: list[int] | None = None,
maybe should comment what continue_steps
means
View / edit / reply to this conversation on ReviewNB
tomtseng commented on 2023-06-09T21:21:13Z ----------------------------------------------------------------
Line #85. dict(
maybe remove opt_return
and always return the dict? the caller can ignore the return value if they don't care about it
View / edit / reply to this conversation on ReviewNB
tomtseng commented on 2023-06-09T21:21:14Z ----------------------------------------------------------------
this comment is in the wrong location
View / edit / reply to this conversation on ReviewNB
tomtseng commented on 2023-06-09T21:21:14Z ----------------------------------------------------------------
Line #8. df_ft["adv_orig_steps"] = df_ft.adv_name.str.split("-cs").str[0].str.split("-os").str[-1].astype(int)
may be helpful to add a comment about what the naming convention for adversaries was for these files — what do cs
and os
mean?
also, what does cml
mean? cumulative? not immediately clear to me what these columns are (though maybe once I see the naming convention for the adversaries it will be easier to figure out)
View / edit / reply to this conversation on ReviewNB
tomtseng commented on 2023-06-09T21:21:15Z ----------------------------------------------------------------
Line #22. drop_steps = steps[drop_left : len(steps) - drop_right]
why do we need to drop items from the beginning and end? a comment on what this cell is doing would be helpful
why drop the change from v32 to v64 at s26985728 for tony-cyc-adv-ft-vs-b60-s7702m-20230518-185923?
Added a comment.
View / edit / reply to this conversation on ReviewNB
tomtseng commented on 2023-06-09T21:21:16Z ----------------------------------------------------------------
text here seems outdated
ed1d1a8d commented on 2023-06-20T17:14:27Z ----------------------------------------------------------------
Updated text.
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB