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.

Sign up for GitHub

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

Jump to bottom

feat: making static analysis as the default option for the TEE #274

Merged
merged 5 commits into from
Jan 15, 2025
Merged

feat: making static analysis as the default option for the TEE #274

merged 5 commits into from
Jan 15, 2025

Conversation

MarIniOnz
Copy link
Contributor

By setting the time_attribute to None, there is no consideration for time dynamics now in the TEE, just whether there are any edges between the treatments, outcomes and the patient nodes.

@MarIniOnz MarIniOnz requested review from JabobKrauskopf and a team as code owners December 5, 2024 11:32
@MarIniOnz MarIniOnz linked an issue Dec 5, 2024 that may be closed by this pull request
feat: adding treatment effect static analysis #273
Closed
LauraBoenchenLB
LauraBoenchenLB requested changes Dec 5, 2024
View reviewed changes
Copy link
Contributor

@LauraBoenchenLB LauraBoenchenLB left a comment

Choose a reason for hiding this comment

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

Great implementation! Needs a few tweaks here and there, but the basis will be good for simple TEEs

medmodels/treatment_effect/continuous_estimators.py Outdated Show resolved Hide resolved
medmodels/treatment_effect/continuous_estimators.py Outdated Show resolved Hide resolved
medmodels/treatment_effect/continuous_estimators.py Outdated Show resolved Hide resolved
medmodels/treatment_effect/continuous_estimators.py Outdated Show resolved Hide resolved
medmodels/treatment_effect/continuous_estimators.py Outdated Show resolved Hide resolved
medmodels/treatment_effect/continuous_estimators.py Outdated Show resolved Hide resolved
medmodels/treatment_effect/continuous_estimators.py Outdated Show resolved Hide resolved
medmodels/treatment_effect/continuous_estimators.py Show resolved Hide resolved
medmodels/treatment_effect/treatment_effect.py Show resolved Hide resolved
@MarIniOnz MarIniOnz force-pushed the 273-feat-adding-treatment-effect-static-analysis branch from e819734 to 752b837 Compare December 9, 2024 13:59
LauraBoenchenLB
LauraBoenchenLB requested changes Dec 17, 2024
View reviewed changes
Copy link
Contributor

@LauraBoenchenLB LauraBoenchenLB left a comment

Choose a reason for hiding this comment

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

Just one more request if possible :)

medmodels/treatment_effect/treatment_effect.py Outdated Show resolved Hide resolved
medmodels/treatment_effect/treatment_effect.py Show resolved Hide resolved
medmodels/treatment_effect/continuous_estimators.py Outdated Show resolved Hide resolved
@MarIniOnz MarIniOnz force-pushed the 273-feat-adding-treatment-effect-static-analysis branch from 752b837 to c154958 Compare December 18, 2024 11:45
@MarIniOnz MarIniOnz force-pushed the 273-feat-adding-treatment-effect-static-analysis branch from c154958 to f997191 Compare January 7, 2025 09:55
LauraBoenchenLB
LauraBoenchenLB previously approved these changes Jan 9, 2025
View reviewed changes
Copy link
Contributor

@LauraBoenchenLB LauraBoenchenLB left a comment

Choose a reason for hiding this comment

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

Great job! I think all cases are handled well now in the tests and the static option will make a simple analysis much easier! 🦭

JabobKrauskopf
JabobKrauskopf requested changes Jan 15, 2025
View reviewed changes
Copy link
Member

@JabobKrauskopf JabobKrauskopf left a comment

Choose a reason for hiding this comment

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

Only super minor stuff, will approve when fixed

medmodels/treatment_effect/builder.py Outdated Show resolved Hide resolved
JabobKrauskopf
JabobKrauskopf approved these changes Jan 15, 2025
View reviewed changes
Copy link
Member

@JabobKrauskopf JabobKrauskopf left a comment

Choose a reason for hiding this comment

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

Screen Shot 2025-01-15 at 12 41 22

@JabobKrauskopf JabobKrauskopf merged commit 816ee6f into main Jan 15, 2025
6 checks passed
@JabobKrauskopf JabobKrauskopf deleted the 273-feat-adding-treatment-effect-static-analysis branch January 15, 2025 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JabobKrauskopf JabobKrauskopf JabobKrauskopf approved these changes

@LauraBoenchenLB LauraBoenchenLB LauraBoenchenLB left review comments

@OFranke OFranke Awaiting requested review from OFranke OFranke is a code owner automatically assigned from limebit/medmodels-software-engineering

@FloLimebit FloLimebit Awaiting requested review from FloLimebit FloLimebit is a code owner automatically assigned from limebit/medmodels-software-engineering

@philippgrosser philippgrosser Awaiting requested review from philippgrosser philippgrosser is a code owner automatically assigned from limebit/medmodels-data-science-team

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: adding treatment effect static analysis
3 participants
@MarIniOnz @JabobKrauskopf @LauraBoenchenLB