-
Notifications
You must be signed in to change notification settings - Fork 230
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
tests: Add test for using a Constant as a condition in a ConditionalDimension #2512
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2512 +/- ##
=======================================
Coverage 87.28% 87.28%
=======================================
Files 238 238
Lines 45724 45733 +9
Branches 4058 4058
=======================================
+ Hits 39911 39920 +9
Misses 5128 5128
Partials 685 685 ☔ View full report in Codecov by Sentry. |
def test_constant_as_condition(self, value): | ||
x = Dimension('x') | ||
|
||
c = Constant(name="c", dtype=np.int8, value=value) |
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.
Shouldn't this be a Bool if it's a condition by itself?
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.
Actually you can enhance the test with a bool as well? Behaviour should be the same?
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.
But C doesn't have a core bool type? You have to import stdbool.h
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.
Convention is to use an int iirc
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.
yes, int is OK
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.
int8 btw is converted into a char
|
||
c = Constant(name="c", dtype=np.int8, value=value) | ||
cd = ConditionalDimension(name="cd", parent=x, condition=c) | ||
|
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.
can probably drop this empty line
def test_constant_as_condition(self, value): | ||
x = Dimension('x') | ||
|
||
c = Constant(name="c", dtype=np.int8, value=value) |
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.
Actually you can enhance the test with a bool as well? Behaviour should be the same?
1963edf
to
909c0e0
Compare
909c0e0
to
4715350
Compare
This case is valid but not currently tested. It is useful for creating switches that can be set in Python-land. Example applications include preventing accesses on empty ranks when performing sparse operations on functions defined on subdomains.