-
Notifications
You must be signed in to change notification settings - Fork 65
Handling for module-level dimension variables for interstitial variables #683
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
Conversation
climbfuji
left a comment
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.
Looks reasonable to me. And I agree that if the dimension isn't set, we'll get a runtime error. Do you think the auto-generated code that is added with --debug will catch it in the caps?
| adjust_intent=False): | ||
| def add_variable_dimensions(self, var, ignore_sources, suite_type, | ||
| to_dict=None, adjust_intent=False): | ||
| """Attempt to find a source for each dimension in <var> and add that |
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 you update the docstring please?
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.
updated!
|
@climbfuji good question! I tested this by moving the setting of the dim_inter variable in the capgen test to the |
Great, thanks for testing! |
gold2718
left a comment
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.
I think this is okay.
|
@mkavulich I believe this can be merged? unless I missed a discussion about this at the last framework meeting |
Updates to allow for dimension variables that are set in the register phase to be used to allocate interstitial variables.
Open to feedback, of course! There's no explicit handling right now to ensure that the variable is set in the register phase, but my feeling is that you'll get a run-time error if you try to use an uninitialized variable in an
allocatestatement.User interface changes?: No
Fixes:
closes #681
Testing: Added variables/logic in capgen test to confirm the dimensions are initialized and used properly