Skip to content
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

Task/jupyter extract #344

Merged
merged 7 commits into from Aug 14, 2019
Merged

Task/jupyter extract #344

merged 7 commits into from Aug 14, 2019

Conversation

SeifIbrahim
Copy link
Collaborator

No description provided.

@cyrush
Copy link
Member

cyrush commented Aug 13, 2019

looks great!

Here are a couple of small shifts we should do pre-merge:

mv ascent_bridge (the client) up to src/ascent/, name the dir ascent_jupyter_bridge

rename py_src/server/ (and the equiv mpi ver) 'py_src/bridge_kernel/' and wire in to reflect the changes

and change example.py to be called server.py

Add yourself to the contribs list:
https://github.com/Alpine-DAV/ascent/blob/develop/src/docs/sphinx/index.rst

Also can we add a link to the bridge kernel under:

https://github.com/Alpine-DAV/ascent/blob/develop/src/docs/sphinx/Licenses.rst

Copy link
Member

@mclarsen mclarsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a number of TODOs. Are they on the wish list or are any of them on the critical path? Its ok if they are not on the critical path.

The camera helpers like rotate are defined in two different files. Can they be merged into a single file?

return info["actions"]

#TODO store MPIServer in global variable instead of passing it
#TODO why doesn't if rank==0 at the top work?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still the case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was experiencing some weird behaviour with openmpi when I wrote that comment, we eventually switched to mpich for testing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it turns out this is still an issue, if I check rank==0 at the top the function doesn't seem to run at all.

if data["type"] == "transform":
call_obj = data["code"]

#TODO better way to do this so i don't have to import everything into this file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better way so you don't have to import everything into this file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I was having with this was that I am sending over a string from the client to tell the server what function it should run and with what arguments. I was sending a string like 'numpy.pi/2' that was needing to be evaled before being passed as an argument, but in order to eval it I had to import numpy into this file. What I might end up doing is instead of evaling the arguments in the 'dispatcher' that calls the function, I'll probably leave more of that functionality up to the function.

@mclarsen
Copy link
Member

mclarsen commented Aug 13, 2019

One item I forgot: there are a bunch of notebook files called untitled. Should they get names?

@SeifIbrahim
Copy link
Collaborator Author

SeifIbrahim commented Aug 13, 2019

There are a number of TODOs. Are they on the wish list or are any of them on the critical path? Its ok if they are not on the critical path.

The camera helpers like rotate are defined in two different files. Can they be merged into a single file?

The TODOs are mostly things that I want to be working eventually (they're mostly related to my trackball widget working nicely) but aren't super critical.

All the code for the bridge kernel server is duplicated at the moment since we have MPI and non-MPI versions for the ascent python module. We might end up merging them into a module separate from the ascent module.

@cyrush cyrush merged commit d9b06bb into develop Aug 14, 2019
@mclarsen mclarsen deleted the task/jupyter_extract branch March 22, 2020 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants