-
Notifications
You must be signed in to change notification settings - Fork 27
Adds persis_in
, removes alloc_specs['in']
, various small refactors
#670
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
…tent func lifetime. removes alloc_specs['in']. refactors many funcs and tests for changes
Pull Request Test Coverage Report for Build 1283944417
💛 - Coveralls |
persis_in
, removes alloc_specs['in'], various small refactorspersis_in
, removes alloc_specs['in']
, various small refactors
persis_info['fields_to_give_back'] = ['f'] + [n[0] for n in gen_specs['out']] | ||
persis_info['fields_to_give_back'] = gen_specs['persis_in'] | ||
|
||
if 'grad' in [n[0] for n in sim_specs['out']]: |
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.
@jmlarson1 @jlnav Do we want to construct fields to give back like this - or expect the user to do it in calling script when creating gen_specs['persis_in'] ? An alloc could support a default, if the user does not set persis_in maybe, but seems to me potentially confusing to mix user supplied and add to it in alloc.
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.
Yeah, I'd lean towards expecting the user to define their fields in their calling script. I think I was more explicit with all the other cases except the new functions from the student.
Any major objections to refactoring the above conditional fields to be in their corresponding calling scripts instead?
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 also think that we should expect the user to define their fields in their calling script.
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.
Sounds good to me.
No description provided.