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

RS/YJ/Rule 23-5 (new) #1259

Merged
merged 10 commits into from
Jan 18, 2024
Merged

RS/YJ/Rule 23-5 (new) #1259

merged 10 commits into from
Jan 18, 2024

Conversation

yunjoonjung-PNNL
Copy link
Collaborator

This rule has been tested with Juan's TCD. The old 23-5 was closed and this new PR was made due to the unexpected many changes in the old PR. Thanks.

@yunjoonjung-PNNL
Copy link
Collaborator Author

For some reason, 22-29 failed. I'll look into this soon.

@weilixu
Copy link
Collaborator

weilixu commented Jan 15, 2024

For some reason, 22-29 failed. I'll look into this soon.

@yunjoonjung-PNNL The 22-29 rule tests are missing "type" field in the condenser loop.
The type shall be Condenser. The bug was identified in an integrated test and a commit addressed it in the rule itself but the tests were not updated yet.

@yunjoonjung-PNNL
Copy link
Collaborator Author

For some reason, 22-29 failed. I'll look into this soon.

@yunjoonjung-PNNL The 22-29 rule tests are missing "type" field in the condenser loop. The type shall be Condenser. The bug was identified in an integrated test and a commit addressed it in the rule itself but the tests were not updated yet.

I pulled #1260 and all the CI/CD tests passed!

Copy link
Collaborator

@weilixu weilixu left a comment

Choose a reason for hiding this comment

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

Good use of flatten.
One comment must be addressed

@weilixu weilixu merged commit 6be59a5 into develop Jan 18, 2024
2 checks passed
@yunjoonjung-PNNL yunjoonjung-PNNL deleted the RS/YJ/Rule_23-5_new branch April 16, 2024 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants