Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
NEAMS Workbench #85
base: development
Are you sure you want to change the base?
NEAMS Workbench #85
Changes from 18 commits
ec4b451
ccafae0
02a7498
14f047e
3edd695
ec8b6fe
7e983aa
b71c8e2
98a7476
fe294b0
5334fb6
72ac4f9
043e314
dc27ddb
68e7ffe
d722a62
b8f9d93
4ae5ad7
6dd0eb4
193c444
50abc5e
0d09d32
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
the setup worked great - very straightforward.
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 don't understand why this directory would need to be defined.
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.
Removed
workflow_dir
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.
We need a
plugin.tmpl
template that has all these options listed out that a user can autocomplete.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.
it would be nice to have list of options, if required or optional, etc. In fact, I can show you how such documentation can be automatically generated with Workbench (and maybe added to this documentation)
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.
We should develop different param templates that show the different options (param.value.tmpl, param.list.tmpl, param.func.tmpl, param.bool.tmpl, etc.)
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.
not sure why plugin_main and plugin_sub needs specifying - I would have guessed we would use the plugin specified under workflow_level1. I see why this is needed currently because we are specifying how data is transferred from 1 app to the other. However, could this be done directly at the workflow/plugin/params level so that it can generalized in case we have additional apps involved in the workflow?
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.
No longer need to specify
plugin_main
but still need to specifyplugin = ID2
. This is specifically foriteration
only because of how the iteration workflow is set up currently. I think it's less confusing this way but we can change it if needed.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.
It is not clear at this point how "keff" is defined (as an output of an application).
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.
Added a few sentences to explain what
keff
and better explanations on how to useiteration
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.
change names of the files -
watts_comprehensive.son
is just an example of input with all options listed out for testing. This file should be morewatts_wb.son
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.
Renamed all .son files
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 don't think providing this link should be necessary! the Python script should know the path to this input already.
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.
Removed
workflow_dir