Kitware / HPCCloud

A Cloud/Web-Based Simulation Environment
https://kitware.github.io/HPCCloud/
Apache License 2.0
50 stars 23 forks source link

Add NWChem NEB workflow #554

Closed cjh1 closed 7 years ago

codecov-io commented 7 years ago

Current coverage is 61.84% (diff: 0.00%)

Merging #554 into master will not change coverage

@@             master       #554   diff @@
==========================================
  Files            59         59          
  Lines          2786       2786          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           1723       1723          
  Misses         1063       1063          
  Partials          0          0          

Powered by Codecov. Last update c24db28...a3cee9c

TristanWright commented 7 years ago

The changes look good to me, cool that we're getting Candela in here.

cjh1 commented 7 years ago

@TristanWright Thanks for the review. One think I would like to do is add the SimPut commands to the build. Currently I am just checking in results, it would be better to actually invoke SimPut as part of the npm install. Would that me possible?

TristanWright commented 7 years ago

Yes, in the scripts section of the package.json you can add a simput command, then tack that command on the postinstall script.

On Fri, Nov 11, 2016 at 11:09 AM, Chris Harris notifications@github.com wrote:

@TristanWright https://github.com/TristanWright Thanks for the review. One think I would like to do is add the SimPut commands to the build. Currently I am just checking in results, it would be better to actually invoke SimPut as part of the npm install. Would that me possible?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Kitware/HPCCloud/pull/554#issuecomment-260018643, or mute the thread https://github.com/notifications/unsubscribe-auth/AMs36UfhTTF5UESQr9goX8HOJsAIIr2Rks5q9K8_gaJpZM4Kt4J8 .

R&D Engineer Kitware Inc.

cjh1 commented 7 years ago

Which cumulus branch are you on?

On Fri, Nov 11, 2016 at 3:31 PM, Chet Nieter notifications@github.com wrote:

@chetnieter requested changes on this pull request.

In server/taskflows/hpccloud/taskflow/nwchem/init.py https://github.com/Kitware/HPCCloud/pull/554#pullrequestreview-8261646:

@@ -76,8 +87,6 @@ def create_geometry_symlink(task, job, cluster, fileName): @cumulus.taskflow.task def setup_input(task, _args, *_kwargs): input_folder_id = kwargs['input']['folder']['id']

So I tried testing this with the development VM and I am now getting the following error when trying to run a NWChem simulation. I added a print statement to check kwargs and found that it is empty. I confirmed that things are still working in master.

[15:26:42.209] ERROR: Exception raise by task. File "/usr/local/lib/python2.7/dist-packages/celery/app/trace.py", line 240, in trace_task R = retval = fun(_args, _kwargs) File "/usr/local/lib/python2.7/dist-packages/celery/app/trace.py", line 438, in protected_call return self.run(_args, _kwargs) File "cumulus/taskflow/init.py", line 117, in wrapped return func(celery_task, _args, *_kwargs) File "/opt/hpccloud/hpccloud/server/taskflows/hpccloud/taskflow/nwchem/init.py", line 89, in setup_input input_folder_id = kwargs['input']['folder']['id'] KeyError: 'input'

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Kitware/HPCCloud/pull/554#pullrequestreview-8261646, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-7ja6uK1iUraWQsmFo4pHLq28RpzBhks5q9NC2gaJpZM4Kt4J8 .

chetnieter commented 7 years ago

master

Ahh - right. I need to checkout your branch since it hasn’t been merged yet.

On Nov 11, 2016, at 3:57 PM, Chris Harris notifications@github.com wrote:

Which cumulus branch are you on?

On Fri, Nov 11, 2016 at 3:31 PM, Chet Nieter notifications@github.com wrote:

@chetnieter requested changes on this pull request.

In server/taskflows/hpccloud/taskflow/nwchem/init.py https://github.com/Kitware/HPCCloud/pull/554#pullrequestreview-8261646:

@@ -76,8 +87,6 @@ def create_geometry_symlink(task, job, cluster, fileName): @cumulus.taskflow.task def setup_input(task, _args, *_kwargs): input_folder_id = kwargs['input']['folder']['id']

So I tried testing this with the development VM and I am now getting the following error when trying to run a NWChem simulation. I added a print statement to check kwargs and found that it is empty. I confirmed that things are still working in master.

[15:26:42.209] ERROR: Exception raise by task. File "/usr/local/lib/python2.7/dist-packages/celery/app/trace.py", line 240, in trace_task R = retval = fun(_args, _kwargs) File "/usr/local/lib/python2.7/dist-packages/celery/app/trace.py", line 438, in protected_call return self.run(_args, _kwargs) File "cumulus/taskflow/init.py", line 117, in wrapped return func(celery_task, _args, *_kwargs) File "/opt/hpccloud/hpccloud/server/taskflows/hpccloud/taskflow/nwchem/init.py", line 89, in setup_input input_folder_id = kwargs['input']['folder']['id'] KeyError: 'input'

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Kitware/HPCCloud/pull/554#pullrequestreview-8261646, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-7ja6uK1iUraWQsmFo4pHLq28RpzBhks5q9NC2gaJpZM4Kt4J8 .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Kitware/HPCCloud/pull/554#issuecomment-260053860, or mute the thread https://github.com/notifications/unsubscribe-auth/AEItTzQ7wkVIhGAoEhnO7kig3-x3cZumks5q9Na4gaJpZM4Kt4J8.

chetnieter commented 7 years ago

Hey Guys - what is the status of this? Can we merge once the integration failures and conflicts are worked out?

cjh1 commented 7 years ago

Yes, absolute, it just been sitting here as I haven't had a chance to get back to it.

cjh1 commented 7 years ago

@chetnieter Do you have time rebase and fix up the CI issues?

chetnieter commented 7 years ago

Yes - I will take a look at the conflicts.

cjh1 commented 7 years ago

Many thanks!

chetnieter commented 7 years ago

@cjh1 Should cumulus and girder be on master or do they need to be on another branch for me to test this?

cjh1 commented 7 years ago

cumulus needs to be on 'chain'

chetnieter commented 7 years ago

@cjh1 @TristanWright So after I merged in master with the conflicts resolved all the integration checks pass. I assume that the chain branch of cumulus will be to merged as well right?

cjh1 commented 7 years ago

@chetnieter Yes, we need to merge that branch too, can you approve it