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

Gather eval stats #157

Closed
wants to merge 9 commits into from
Closed

Gather eval stats #157

wants to merge 9 commits into from

Conversation

reaganjlee
Copy link
Collaborator

Adds in a new function get_metric_across_layers that can be used to get a metric score of a reporter across layers automatically.
Some questions/things to work on

  1. I'm looking to make this callable from CLI with something similar to elk gather (open to thoughts on the command)
  2. The function requires that there is a reporter directory to run the tests. I test it by having a reporter directory and setting ENV_DIR to that, but am unsure on how to test this with pytest on the library by itself

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.

Thanks for your pull request! I've added some comments

"""
REPORTER_DIR = os.path.abspath(reporter_dir)

if not os.path.exists(REPORTER_DIR):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need to create the reporter_dir first?

@@ -0,0 +1,56 @@
import pandas as pd
import numpy as np
import matplotlib.pyplot as plt
Copy link
Collaborator

Choose a reason for hiding this comment

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

there are missing imports

@@ -24,7 +24,7 @@

# These are users whose datasets should be included in the results returned by
# filter_english_datasets (regardless of their metadata)
INCLUDED_USERS = {"Zaid", "craffel", "lauritowal"}
INCLUDED_USERS = {"Zaid", "craffel", "lauritowal", "christykoh"}
Copy link
Collaborator

@lauritowal lauritowal Mar 31, 2023

Choose a reason for hiding this comment

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

You should remove the template changes, since this is not related to visualizing the stats (Same for all the template files / directories)

@CLAassistant
Copy link

CLAassistant commented Apr 23, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ ChristyKoh
❌ reaganjlee
You have signed the CLA already but the status is still pending? Let us recheck it.

@norabelrose
Copy link
Member

Not clear what this is doing; happy to look at an updated version in the future

@reaganjlee reaganjlee deleted the gather-eval-stats branch August 18, 2023 07:54
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