NBS Test Cases #123

cbrinson-rise8 · 2024-11-08T21:12:48Z

Description

NBS provided us a spreadsheet that defines an existing state and then a bunch of tests with expected results. Architect a testing script of sorts, that can parse a similar spreadsheet (can just be a CSV) with seeding data and iterate through the tests with the expectations.

Related Issues

closes #110

Additional Notes

For a full explanation of run instructions, directory structure, and file uses check out the README.md

The key inputs to these tests will be the following:

a CSV file that can be used to seed the MPI
a CSV with test cases (basically the same format as the above), but also a column to indicate if it is a match and to whom
a JSON file with an algorithm config to use
The setup should make it easy to edit any 3 of those above files, run the test cases, and examine the results.

<--------------------- REMOVE THE LINES BELOW BEFORE MERGING --------------------->

Checklist

Please review and complete the following checklist before submitting your pull request:

I have ensured that the pull request is of a manageable size, allowing it to be reviewed within a single session.
I have reviewed my changes to ensure they are clear, concise, and well-documented.
I have updated the documentation, if applicable.
I have added or updated test cases to cover my changes, if applicable.
I have minimized the number of reviewers to include only those essential for the review.

Checklist for Reviewers

Please review and complete the following checklist during the review process:

The code follows best practices and conventions.
The changes implement the desired functionality or fix the reported issue.
The tests cover the new changes and pass successfully.
Any potential edge cases or error scenarios have been considered.

codecov · 2024-11-08T21:14:52Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.43%. Comparing base (3182485) to head (f8d811b).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #123      +/-   ##
==========================================
- Coverage   95.47%   95.43%   -0.04%     
==========================================
  Files          31       31              
  Lines        1414     1424      +10     
==========================================
+ Hits         1350     1359       +9     
- Misses         64       65       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tests/algorithm/scripts/seed_db.py

tests/algorithm/scripts/run_test.py

src/recordlinker/linking/mpi_service.py

tests/algorithm/scripts/send_test_records.py

ericbuckley · 2024-11-21T19:40:43Z

tests/algorithm/scripts/send_test_records.py

+
+
+def send_test_records(test_csv, algorithm_name, api_url):
+    df = pd.read_csv(test_csv)


Is pandas only necessary to read and write csv files? If so, there is a standard lib for that https://docs.python.org/3/library/csv.html

Yeah. I will go ahead and switch it to that standard lib. Removing it should make the build faster too.

tests/algorithm/scripts/run_test.py

tests/algorithm/README.md

tests/algorithm/results/output.csv

ericbuckley · 2024-11-21T23:22:45Z

@cbrinson-rise8 I was going through the README and ran the default tests. I noticed the all of the expected results in the output were either a failure or a match, which is different from the results.csv file you committed. Any ideas why my results would have been different from the ones in the PR?

cbrinson-rise8 · 2024-11-26T15:30:08Z

@cbrinson-rise8 I was going through the README and ran the default tests. I noticed the all of the expected results in the output were either a failure or a match, which is different from the results.csv file you committed. Any ideas why my results would have been different from the ones in the PR?

Hmm not sure but the output file I committed might just be old. I'll run through running the tests and see what my new output file would look like.

alhayward

Thanks for this work! Excited that we'll have the functionality to test results, which paves the way for calculating match accuracy performance metrics!

Left several questions and a few suggestions.

alhayward · 2024-11-27T01:08:40Z

tests/algorithm/README.md

@@ -0,0 +1,99 @@
+# Record Linkage Algorithm Testing
+
+This repository contains a project to test the effectiveness of the RecordLinker algorithm.


Suggested change

This repository contains a project to test the effectiveness of the RecordLinker algorithm.

This repository contains a project to evaluate the match accuracy performance of the RecordLinker algorithm.

alhayward · 2024-11-27T01:09:56Z

tests/algorithm/README.md

+- `results/`: Contains the results of the algorithm tests
+- `scripts/`: Contains the scripts to run the algorithm tests
+
+## Steup


Suggested change

## Steup

## Setup

alhayward · 2024-11-27T01:12:32Z

tests/algorithm/README.md

+
+    The results of the algorithm tests will be available in the `results/output.csv` file.
+
+    The results will be in a csv formatted file withthe following columns: `Test Case #`, `Expected Result`, `Match Results`, and `Error`.


Suggested change

The results will be in a csv formatted file withthe following columns: `Test Case #`, `Expected Result`, `Match Results`, and `Error`.

The results will be in a CSV formatted file with the following columns: `Test Case #`, `Expected Result`, `Match Results`, and `Error`.

We may want to edit these column names over time to align with naming conventions of performance metrics used for evaluating classification models, like the Record Linkage algorithm (3 classes: match, no_match, possible_match). E.g. True Label instead of Expected Result and Prediction instead of Match Results.

alhayward · 2024-11-27T01:22:29Z

tests/algorithm/configurations/algorithm_configuration.json

+            "evaluators": [
+                    {
+                        "feature": "FIRST_NAME",
+                        "func": "func:recordlinker.linking.matchers.feature_match_fuzzy_string"


Depending on when this gets merged, you may need to pull the latest changes from main with the new names of the feature matching/evaluator funcs, if it's after #137 is merged.

alhayward · 2024-11-27T01:23:32Z

tests/algorithm/data/seed_data.csv

@@ -0,0 +1,1001 @@
+Match Id,ID,BIRTHDATE,FIRST,LAST,SUFFIX,MAIDEN,RACE,ETHNICITY,GENDER,ADDRESS,CITY,STATE,COUNTY,ZIP,SSN,


Prob fine since this is synthetic data, but do we need approval from NBS to store this on our open-source GitHub repo? CC: @ericbuckley

alhayward · 2024-11-27T01:26:36Z

tests/algorithm/scripts/seed_db.py

+
+
+def seed_database(csv_file, api_url):
+    MAX_CLUSTERS = 100


What is the purpose of constraining the number of clusters to 100?

alhayward · 2024-11-27T01:28:06Z

tests/algorithm/scripts/send_test_records.py

+def send_test_records(test_csv, algorithm_name, api_url):
+    output_data = []
+
+    print("Sending test records to the API...")


I assume these messages get printed to the CLI? What's the difference between printing vs. logging in this case?

alhayward · 2024-11-27T01:30:42Z

tests/algorithm/scripts/send_test_records.py

+                output_row = {
+                    "Test Case #": match_info['test_case_number'],
+                    "Expected Result": match_info['should_match'],
+                    "Match Result": response['prediction'],


Our valid prediction values of "match", "no_match", and "possible_match" don't exactly align with those in the Expected Result column of the NBS test cases. Is there transformation somewhere, or are we expecting our valid values in this column? If the latter, is validation somewhere needed?

For context, having the same set of values in the Expected Result and Match Result columns of the output.csv is very helpful for calculating match accuracy performance metrics down the line (e.g., False Positives, False Negatives, Sensitivity, Specificity, etc.). If there are transformations to results needed, I think that should live somewhere in this testing framework and outside of the calculations for these metrics.

E.g.,"Should Be a Match" in NBS test cases vs. "match" in Record Linkage algorithm prediction, "Should Be a Match?" vs. "possible_match", etc.

alhayward · 2024-11-27T01:34:04Z

tests/algorithm/scripts/send_test_records.py

+                output_row = {
+                    "Test Case #": match_info['test_case_number'],
+                    "Expected Result": match_info['should_match'],
+                    "Match Result": "Failed",


What would cause this "Failed" result?

Would we want to error out in the testing framework somewhere earlier on? Not sure how long this kind of testing would take to run, but I'm imaging the case when it takes a while only to return many "Failed to link record" results due to a formatting error or something easily fixed; the user would only become aware of that after the testing finished running.

cbrinson-rise8 self-assigned this Nov 8, 2024

cbrinson-rise8 added the qa Technical improvements to increase code quality label Nov 8, 2024

cbrinson-rise8 force-pushed the feat/add-nbs-test-suite branch 2 times, most recently from 7377609 to aa66f56 Compare November 14, 2024 17:13

github-advanced-security bot found potential problems Nov 14, 2024

View reviewed changes

tests/algorithm/scripts/seed_db.py Fixed Show fixed Hide fixed

tests/algorithm/scripts/seed_db.py Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Nov 15, 2024

View reviewed changes

tests/algorithm/scripts/run_test.py Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Nov 18, 2024

View reviewed changes

tests/algorithm/scripts/run_test.py Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Nov 18, 2024

View reviewed changes

tests/algorithm/scripts/run_test.py Fixed Show fixed Hide fixed

cbrinson-rise8 commented Nov 19, 2024

View reviewed changes

src/recordlinker/linking/mpi_service.py Outdated Show resolved Hide resolved

cbrinson-rise8 force-pushed the feat/add-nbs-test-suite branch 2 times, most recently from c36a188 to 24e7026 Compare November 19, 2024 19:04

github-advanced-security bot found potential problems Nov 19, 2024

View reviewed changes

tests/algorithm/scripts/send_test_records.py Fixed Show fixed Hide fixed

cbrinson-rise8 force-pushed the feat/add-nbs-test-suite branch 2 times, most recently from 7f159f4 to 4038dd1 Compare November 20, 2024 15:08

feat: initial commit

267ef0c

cbrinson-rise8 force-pushed the feat/add-nbs-test-suite branch from 957bdaf to 267ef0c Compare November 21, 2024 15:04

cbrinson-rise8 added 3 commits November 21, 2024 10:13

feat: updated readme

001f30f

feat: readme adjustment

b7bf697

feat: added algorithm updates

6642e02

cbrinson-rise8 marked this pull request as ready for review November 21, 2024 15:59

cbrinson-rise8 requested review from ericbuckley and alhayward as code owners November 21, 2024 15:59

cbrinson-rise8 added 2 commits November 21, 2024 11:00

feat: linting

206a92e

feat: quick fix

9c842e0

ericbuckley reviewed Nov 21, 2024

View reviewed changes

tests/algorithm/scripts/run_test.py Show resolved Hide resolved

ericbuckley reviewed Nov 21, 2024

View reviewed changes

tests/algorithm/README.md Outdated Show resolved Hide resolved

ericbuckley reviewed Nov 21, 2024

View reviewed changes

tests/algorithm/results/output.csv Outdated Show resolved Hide resolved

cbrinson-rise8 added 4 commits November 22, 2024 11:20

feat: change the run commands

a068004

feat: typo

7a49789

feat: update readme

bd80014

feat: removed s

bae7739

feat: comments

f8d811b

alhayward reviewed Nov 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NBS Test Cases #123

NBS Test Cases #123

cbrinson-rise8 commented Nov 8, 2024 •

edited

Loading

codecov bot commented Nov 8, 2024 •

edited

Loading

ericbuckley Nov 21, 2024

cbrinson-rise8 Nov 26, 2024 •

edited

Loading

ericbuckley commented Nov 21, 2024

cbrinson-rise8 commented Nov 26, 2024

alhayward left a comment

alhayward Nov 27, 2024

alhayward Nov 27, 2024

alhayward Nov 27, 2024

alhayward Nov 27, 2024

alhayward Nov 27, 2024

alhayward Nov 27, 2024

alhayward Nov 27, 2024

alhayward Nov 27, 2024

alhayward Nov 27, 2024

alhayward Nov 27, 2024



		def send_test_records(test_csv, algorithm_name, api_url):
		df = pd.read_csv(test_csv)

		@@ -0,0 +1,99 @@
		# Record Linkage Algorithm Testing

		This repository contains a project to test the effectiveness of the RecordLinker algorithm.

	This repository contains a project to test the effectiveness of the RecordLinker algorithm.
	This repository contains a project to evaluate the match accuracy performance of the RecordLinker algorithm.


		The results of the algorithm tests will be available in the `results/output.csv` file.

		The results will be in a csv formatted file withthe following columns: `Test Case #`, `Expected Result`, `Match Results`, and `Error`.

		@@ -0,0 +1,1001 @@
		Match Id,ID,BIRTHDATE,FIRST,LAST,SUFFIX,MAIDEN,RACE,ETHNICITY,GENDER,ADDRESS,CITY,STATE,COUNTY,ZIP,SSN,

NBS Test Cases #123

Are you sure you want to change the base?

NBS Test Cases #123

Conversation

cbrinson-rise8 commented Nov 8, 2024 • edited Loading

Description

Related Issues

Additional Notes

Checklist

Checklist for Reviewers

codecov bot commented Nov 8, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

cbrinson-rise8 Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

ericbuckley commented Nov 21, 2024

cbrinson-rise8 commented Nov 26, 2024

alhayward left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbrinson-rise8 commented Nov 8, 2024 •

edited

Loading

codecov bot commented Nov 8, 2024 •

edited

Loading

cbrinson-rise8 Nov 26, 2024 •

edited

Loading