Skip to content

Minor evaluation changes tk - #1

Open
TuomasKetola wants to merge 3 commits into
alephdata:mainfrom
TuomasKetola:minor_evaluation_changes-tk
Open

Minor evaluation changes tk#1
TuomasKetola wants to merge 3 commits into
alephdata:mainfrom
TuomasKetola:minor_evaluation_changes-tk

Conversation

@TuomasKetola

Copy link
Copy Markdown

Fixed a few things that were not working with imports. Hopefully not just a problem my end.

Added a notebook for elementwise comparisons and an evaluation file to go with it. The notebook has an example comparing the current followthemoney.compare.py to one with messed up weigthts.

@mynameisfiber

Copy link
Copy Markdown
Contributor

Could you take the code in your evaluate.py and encode it as a model method here. You can see there's already a precision_recall_accuracy_curve method which can serve as a good example. I would imagine the method would take the form:

    @classmethod
    def compare_f1_precision_recall(model1, model2, data):
        ...

That way we can easily compare any two model pairs?

# registry.url: 3.211995327345834,
# None: -11.91521189545115,
# }
COMPARE_WEIGHTS = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Grr... I wrote a long comment here but somehow it got lost. Essentially, I don't think we should recreate followthemoney.compare here because the two codebases could diverge. Instead, I think we should keep the new COMPARE_WEIGHTS (and name them COMPARE_WEIGHTS_RANDOM or something so there is no confusion as to what they do) and use followthemoney's compare.compare_scores and compare._compare to create the scores.

That would look like:

from followthemoney import compare
scores = compare.compare_scores(model, entitty1, entity2)
result_ftm = compare._compare(scores, compare.COMPARE_WEIGHTS)
result_ours = compare._compare(scores, COMPARE_WEIGHTS_RANDOM)

That being said, ideally I think this repo should be the arbiter of what comparisons are. Instead of importing from followthemoney, we should probably stick to using our own model objects defined in followthemoney_compare.model. You can see the model_base.py object (which all models inherit from) already defines some utility evaluation methods.

@TuomasKetola

Copy link
Copy Markdown
Author

Could you take the code in your evaluate.py and encode it as a model method

I'll change how the weights are used as well. I think it might be good to use this PR just to add the evaluation bits. Still not a 100% sure / clear what's the best way to play around with different comparisons, but I'll ask those questions once this is done.

@mynameisfiber

Copy link
Copy Markdown
Contributor

@TuomasKetola going a bit deeper into things with the word frequency normalization, I realize I was wrong and we probably should include a version of followthemoney.compare in this code. I'm making some commits that will include that, so if you rebase you should have access to the methods in followthemoney_compare.compare

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

2 participants