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

Add "checkpoint_mode" kwarg to plotting #99

Merged
merged 5 commits into from
Apr 3, 2024

Conversation

siwei-li
Copy link
Collaborator

No description provided.

@siwei-li siwei-li linked an issue Mar 31, 2024 that may be closed by this pull request
@siwei-li siwei-li marked this pull request as ready for review April 1, 2024 16:46
Copy link
Contributor

@jettjaniak jettjaniak left a comment

Choose a reason for hiding this comment

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

notebook is not running

BeartypeCallHintReturnViolation: Function delphi.eval.vis_per_token_model.visualize_per_token_category.get_plot_values() return (defaultdict(None, {'nouns': (1.370191698609721, 0.4339272244650738, 0.9698742185740588),...)})) violates type hint tuple[numpy.ndarray, numpy.ndarray, numpy.ndarray] under non-default configuration BeartypeConf(claw_is_pep526=True, is_color=None, is_debug=False, is_pep484_tower=False, strategy=<BeartypeStrategy.O1: 2>, warning_cls_on_decorator_exception=<class 'beartype.roar._roarwarn.BeartypeClawDecorWarning'>), as tuple index 0 item <class "collections.defaultdict"> "defaultdict(None, {'nouns': (1.370191698609721, 0.4339272244650738, 0.9698742185740588),...)})" not instance of <protocol "numpy.ndarray">.

@jettjaniak
Copy link
Contributor

  • The idea behind **kwargs was that we may be able to pass some additional arguments to plotly calls without specifying them directly, like plt.line(data, **kwargs).
  • I can see it's hard to make it work that way because we have multiple plotly calls
  • kwargs.get(arg_name, default) doesn't make much sense, we'd be better off just defining arg_name as an argument to the function
  • but if we can't make it simple and configurable, let's just make it simple and non-configurable. so remove the kwargs and add checkpoint_mode argument

@jettjaniak
Copy link
Contributor

also, please remove the commented code

@siwei-li siwei-li force-pushed the 87-make-eval-plot-for-model-checkpoints branch from 963e32b to 8ea8f00 Compare April 1, 2024 23:25
@siwei-li siwei-li requested a review from jettjaniak April 2, 2024 01:29
@jettjaniak jettjaniak force-pushed the 87-make-eval-plot-for-model-checkpoints branch from 8ea8f00 to 2988ddb Compare April 3, 2024 18:38
@jettjaniak jettjaniak merged commit 90410c8 into main Apr 3, 2024
1 check passed
@jettjaniak jettjaniak deleted the 87-make-eval-plot-for-model-checkpoints branch April 3, 2024 18:42
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.

make eval plot for model checkpoints
2 participants