madisonmay / finetune

Scikit-learn style model finetuning for NLP
https://finetune.indico.io
Mozilla Public License 2.0
0 stars 0 forks source link

Sweep: add optional param to SequenceLabeler.predict to allow predicting on predefined spans #1

Open madisonmay opened 1 year ago

madisonmay commented 1 year ago

Is your feature request related to a problem? Please describe. In NER (named entity detection) we sometimes already know the segmentation of the entities, but still need to classify their type. E.g. in the sentece 'Paxar Corp said it has acquired Thermo-Print GmbH' We might know that 'Paxar Corp' and 'Thermo-Print GmbH' are the relevant entities, but we want to predict their label as ORG.

Describe the solution you'd like Perhaps add an optional param named spans to SequenceLabeler.predict, which is a list of dictionaries. Each dictionary will contain the start and end indices.

sweep-ai[bot] commented 1 year ago

Here's the PR! https://github.com/madisonmay/finetune/pull/3.

⚡ Sweep Free Trial: I used GPT-4 to create this ticket. You have 5 GPT-4 tickets left for the month and 2 for the day. For more GPT-4 tickets, visit our payment portal.To get Sweep to recreate this ticket, leave a comment prefixed with "sweep:" or edit the issue.


Step 1: 🔍 Code Search

I found the following snippets in your repository. I will now analyze these snippets and come up with a plan.

Some code snippets I looked at (click to expand). If some file is missing from here, you can mention the path in the ticket description. https://github.com/madisonmay/finetune/blob/be06ddc4b74d30df1227e1ec22af98619bc79fb5/finetune/encoding/sequence_encoder.py#L63-L174 https://github.com/madisonmay/finetune/blob/be06ddc4b74d30df1227e1ec22af98619bc79fb5/finetune/target_models/sequence_labeling.py#L466-L546 https://github.com/madisonmay/finetune/blob/be06ddc4b74d30df1227e1ec22af98619bc79fb5/finetune/encoding/sequence_encoder.py#L193-L302 https://github.com/madisonmay/finetune/blob/be06ddc4b74d30df1227e1ec22af98619bc79fb5/tests/test_sequence.py#L310-L395 https://github.com/madisonmay/finetune/blob/be06ddc4b74d30df1227e1ec22af98619bc79fb5/finetune/nn/target_blocks.py#L368-L475

Step 2: 🧐 Snippet Analysis

From looking through the relevant snippets, I decided to make the following modifications:

File Path Proposed Changes
finetune/target_models/sequence_labeling.py Add an optional parameter named 'spans' to the 'predict' method of the 'SequenceLabeler' class. Update the 'predict' method to handle the case when the 'spans' parameter is provided. In this case, the method should only make predictions for the provided spans. Also, update the '_predict' and '_predict_decode' methods to handle the case when the 'spans' parameter is provided.
finetune/nn/target_blocks.py Update the 'sequence_labeler' method to handle the case when the 'spans' parameter is provided. In this case, the method should only make predictions for the provided spans.

Step 3: 📝 Planning

I have created a plan for writing the pull request. I am now working my plan and coding the required changes to address this issue. Here is the planned pull request:

Add optional param to SequenceLabeler.predict for predefined spans sweep/add-predict-spans-param

Description

This PR adds an optional parameter to the SequenceLabeler.predict method in the SequenceLabeler class. The new parameter, spans, is a list of dictionaries where each dictionary contains the start and end indices of the predefined spans. This feature allows for predicting the labels of predefined spans in scenarios where the segmentation of entities is already known.

Summary of Changes

  • Modified the SequenceLabeler class in finetune/target_models/sequence_labeling.py to include the spans parameter in the predict method.
  • Updated the predict method to handle the case when the spans parameter is provided. The method now only makes predictions for the provided spans.
  • Updated the _predict and _predict_decode methods in finetune/target_models/sequence_labeling.py to handle the case when the spans parameter is provided.
  • Updated the sequence_labeler method in finetune/nn/target_blocks.py to handle the case when the spans parameter is provided.

Step 4: ⌨️ Coding

File Instructions Progress
finetune/target_models/sequence_labeling.py Add an optional parameter named 'spans' to the 'predict' method of the 'SequenceLabeler' class. Update the 'predict' method to handle the case when the 'spans' parameter is provided. In this case, the method should only make predictions for the provided spans. Also, update the '_predict' and '_predict_decode' methods to handle the case when the 'spans' parameter is provided. ✅ Commit be06ddc
finetune/nn/target_blocks.py Update the 'sequence_labeler' method to handle the case when the 'spans' parameter is provided. In this case, the method should only make predictions for the provided spans. ✅ Commit a5d8bd9 I have finished coding the issue. I am now reviewing it for completeness.

Step 5: 🔁 Code Review

Here are the my self-reviews of my changes at sweep/add-predict-spans-param.

Here is the 1st review

Thanks for your contribution. There are a few changes that need to be made:

  • In finetune/target_models/sequence_labeling.py, you've added a spans parameter to several functions and used it to conditionally call a _predict_spans function. Please ensure that this function is defined elsewhere in the code, otherwise, it will lead to a NameError.

  • Also in finetune/target_models/sequence_labeling.py, the spans parameter is added to the _predict_decode function but it's not used within the function. If it's not needed, consider removing it to avoid confusion. If it is needed, please implement its usage.

Please make these changes and update the pull request. If you need any help, feel free to ask.

I finished incorporating these changes.


To recreate the pull request, leave a comment prefixed with "sweep:" or edit the issue. Join Our Discord