-
Notifications
You must be signed in to change notification settings - Fork 5
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
Refactor/96 improve prompts #138
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know prompts are a highly critical topic, and I've left some suggestions for improvement.
tracex_project/extraction/logic/modules/module_patient_journey_preprocessor.py
Show resolved
Hide resolved
tracex_project/extraction/logic/modules/module_patient_journey_preprocessor.py
Show resolved
Hide resolved
tracex_project/extraction/logic/modules/module_patient_journey_preprocessor.py
Show resolved
Hide resolved
Maybe I should give a general disclaimer for this PR. After a lot of trial and error the prompts now make a lot more sense to a human that reads them. The concept of the preprocessing module is more clearly reflected in the prompts. However, the overall performance of the the preprocessing did not increase in a measurable level. Other factors, such as the patient journey, still have a major impact on the quality of the results |
Should I wait with the review until you implemented Pits requests or should i go straight ahead? |
I implemented the requested changes, aside from those that are still in discussion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still some things left to discuss.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, just one minor request
tracex_project/extraction/logic/modules/module_activity_labeler.py
Outdated
Show resolved
Hide resolved
# Conflicts: # tracex_project/db.sqlite3
This PR improved the prompts of the preprocessor and activity labeler. In order to adapt the other prompts to these changes, the prompts of the time extractor module have also been updated.
The following changes were made…
Preprocessing:
Cohorts:
Activity Labeler:
Time Extractor:
In order to review this PR, please pull (make sure the database is also updated in the process) and test if any of the functionality is broken or scrutinize the prompts in the admin view directly. Determining if the changes are improvements by executing the pipeline has proven to be rather difficult.
This closes #96.