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.

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

RT/JDJ/resolve na undetermined failures #1453

Conversation

JacksonJ-KC
Copy link
Collaborator

@JacksonJ-KC JacksonJ-KC commented Aug 1, 2024

@jugonzal07 @weilixu @yunjoonjung-PNNL This list of rule test failures and associated solutions to the failures is complete. Once the issues are all addressed and merged the CI should pass for this PR .. I would appreciate any help in knocking out the remaining issues.

1-1-e #1452
4-2-b #1440
4-11-d #1440
5-2-a #1442
5-2-b #1442
5-3-a #1422 (comment)
5-5-a #1422 (comment)
5-7-a #1422 (comment)
5-9-a #1422 (comment)
5-11-a #1422 (comment)
5-17-a #1460
5-17-b #1460
5-17-c #1460
5-28-a #1461
5-38-a #1462
6-2-d #1422 (comment)
6-8-d #1445
19-3-a #1455
19-3-b #1455
19-4-d #1466
19-10-h #1458
19-10-i #1458
19-16-a #1459
19-16-b #1459
22-8-c #1422 (comment)
22-9-c #1422 (comment)
22-17-d #1422 (comment)
22-18-c #1422 (comment)
22-19-e #1422 (comment)
22-28-c #1422 (comment)
22-33-d #1454
22-41-c #1450
23-4-a #1465

gonz102 and others added 4 commits July 31, 2024 09:42
…ndetermined_fix' into RT/JDJ/resolve-na-undetermined-failures

# Conflicts:
#	rct229/ruletest_engine/ruletest_engine.py
…ndetermined_fix' into RT/JDJ/resolve-na-undetermined-failures
@JacksonJ-KC JacksonJ-KC self-assigned this Aug 1, 2024
@jugonzal07
Copy link
Contributor

This is a really impressive amount of work @JacksonJ-KC ! I'm not sure how you were able to get so much done. Quite a few had issues that were being missed. Thanks so much for documenting and repairing so many of them.

I'm glad to see there's a common thread in many of them with respect to NOT_APPLICABLE. I'm hopeful that there should be a way to update RuleDefinitionListBase to address those broadly. It'd be great if an empty list resulted in NOT_APPLICABLE broadly, without having to redefine every function. Perhaps folks more familiar with that class have some thoughts.

Alternatively, I could interpret an empty "results" list from the evaluate_rule as NOT_APPLICABLE in the ruletest engine. I'm not sure how folks would feel about that though-- it seems like a recipe for misinterpreting a rule evaluation result.

@JacksonJ-KC
Copy link
Collaborator Author

JacksonJ-KC commented Aug 1, 2024

The empty results failure was critical to identifying some of these issues so I strongly recommend keeping that feature.

@JacksonJ-KC JacksonJ-KC changed the base branch from develop to RT/JG/ruletest_na_undetermined_fix August 5, 2024 14:53
@JacksonJ-KC JacksonJ-KC marked this pull request as ready for review August 5, 2024 15:23
@JacksonJ-KC JacksonJ-KC merged commit 407e3ff into RT/JG/ruletest_na_undetermined_fix Aug 5, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants