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.

Sign up for GitHub

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

Clarify raptor tutorial #3052

Merged
merged 5 commits into from
Oct 11, 2023
Merged

Clarify raptor tutorial #3052

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e805daa
Clarify raptor tutorial
ejjordan Oct 5, 2023
b53934e
address review
Oct 6, 2023
6a0f5d9
Merge branch 'devel' into patch-1
andre-merzky Oct 10, 2023
e00ee17
fix notebook
Oct 11, 2023
0557f5a
Merge branch 'devel' into patch-1
andre-merzky Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions docs/source/tutorials/raptor.ipynb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@
"source": [
Copy link
Member

@andre-merzky andre-merzky Oct 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Line #12.                               'args'     : [1, 2]})

Shouldn't the args be removed here?


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sorry for that. I made a local branch and tried to push directly to the parent repo. When I saw that I didn't have permissions (perhaps such development is discouraged?), I just used the editor on github. I just forgot to also remove this line.

On a related note though, I could not get the args to work. Perhaps some documentation on how to use this is in order?
{'mode' : rp.TASK_FUNCTION, 'function' : 'msg', 'args' : [3]} # Fails

Copy link
Member

Choose a reason for hiding this comment

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

Oh, let me check that! Thanks for noting this!

Copy link
Member

@andre-merzky andre-merzky Oct 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, I was a bit blind and should have seen that quicker. But also it points to a lack of documentation.

What happened is not that the args do not work, but rather that the 'function': 'msg' is the problem. The function msg is not defined in the namespace of the raptor worker, as it only defined in the client code. So the symbolic reference (the string 'msg') cannot be resolved and the call fails. In the earlier version you used (msg(3)), the function (and its arguments!) is/are serialized and passed to the raptor worker and then are available there.

I will open a documentation ticket for this - but I think it has no immediate bearing on the proposed notebook change. (see #3057)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that this notebook change can go forward in parallel to any changes to documentation or implementation of raptor. I just wanted to raise the issue somewhere. It is generally preferable to bring such things up in issues, but I was not sure if there was a way to make it work that I was just missing.

Please let me know if anything more is needed from my end for this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Ack! This looks good to me now. Thanks!

"### Task execution\n",
"\n",
"At this point we have the pilot set up and running, we started the master task, and the master will upon initialization start the worker tasks: the RAPTOR overlay is now ready to execute a Python function. The default RAPTOR Worker has a built-in test function which you can use for testing purpose:\n"
"At this point we have the pilot set up and running, we started the master task, and the master will upon initialization start the worker tasks: the RAPTOR overlay is now ready to execute a Python function. Note that a function must be decorated with the `rp.pythontask` decorator for it to be callable as a raptor function.\n"
]
},
{
Expand All @@ -169,10 +169,17 @@
},
"outputs": [],
"source": [
"# function for raptor to execute\n",
"@rp.pythontask\n",
"def msg(val: int):\n",
" if(val %2 == 0):\n",
" print('Regular message')\n",
" else:\n",
" print(f'This is a very odd message: {val}')\n",
"\n"
ejjordan marked this conversation as resolved.
Show resolved Hide resolved
"# create a minimal executable task\n",
"td = rp.TaskDescription({'mode' : rp.TASK_FUNCTION,\n",
" 'function' : 'test', \n",
" 'args' : [1, 2]})\n",
" 'function' : msg(3)})\n",
"task = raptor.submit_tasks([td])[0]"
]
},
Expand Down
Loading