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

[Relax] Additional unit tests for RemoveUnusedParameters #16574

Merged
merged 2 commits into from Feb 23, 2024

Conversation

Lunderberg
Copy link
Contributor

Verifying behavior for subroutines that receive R.Prim or R.Shape parameters, if the symbolic variables defined by those parameters are already defined by another parameter.

Verifying behavior for subroutines that receive `R.Prim` or `R.Shape`
parameters, if the symbolic variables defined by those parameters are
already defined by another parameter.
@Lunderberg
Copy link
Contributor Author

This test case was written to verify the statements I made in this comment. Adding them to the CI, as it was behavior of RemoveUnusedParameters that wasn't previously being tested.

class TestRemoveExtraShapeVariables(BaseCompare):
"""Remove parameters that only serve to define existing symbolic variables

If a `R.Shape` parameter provies a definition of a symbolic
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If a `R.Shape` parameter provies a definition of a symbolic
If a `R.Shape` parameter provides a definition of a symbolic

typo 😜

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, and fixed!

Copy link
Contributor

@slyubomirsky slyubomirsky left a comment

Choose a reason for hiding this comment

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

Nice to have more tests, and thank you for including explanations of what the test cases demonstrate.

@Lunderberg Lunderberg merged commit faa6628 into apache:main Feb 23, 2024
20 checks passed
@Lunderberg Lunderberg deleted the unittest_remove_unused_parameters branch February 23, 2024 14:06
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.

None yet

2 participants