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

Add qprocess job launching #328

Merged
merged 15 commits into from
Mar 6, 2024
Merged

Add qprocess job launching #328

merged 15 commits into from
Mar 6, 2024

Conversation

MBartkowiakSTFC
Copy link
Collaborator

Description of work
We switch from QThread to multiprocessing.Process for starting new jobs. This way the jobs will not be competing for the Python GIL.

Fixes
A State pattern has been implemented for jobs.
A context menu lets the user pause, restart and terminate jobs from the GUI.
Each job is a process and uses pipes to communicate with the main process.

To test
Run a few longer jobs at the same time. See if they all use 100% of a CPU core. Try pausing and resuming them.

@ChiCheng45
Copy link
Collaborator

A few issues I've found from my testing so far.

  • For an non-technical user they probably won't know the exact differences between kill and terminate. Might be just better to have just one of them.
  • Closing the MDANSE main window leaves jobs running. This would be that the user would need to kill the job manually in the task manager.
  • Job kill/pause does seem to work correctly with trajectory conversion jobs.

Animation9

  • Percentage complete can go over 100% in some cases.

image

@MBartkowiakSTFC
Copy link
Collaborator Author

Thanks for finding these problems!
Regarding the jobs that keep running after the GUI has been closed: do they finish normally, and produce correct results?

@ChiCheng45
Copy link
Collaborator

ChiCheng45 commented Feb 28, 2024

Thanks for finding these problems! Regarding the jobs that keep running after the GUI has been closed: do they finish normally, and produce correct results?

I just tried it with a trajectory conversion and I closed the main window before it finished. After waiting some time it produced an MDT file which loads successfully with a new MDANSE.

So yea it appears to finish successfully.

@MBartkowiakSTFC
Copy link
Collaborator Author

That's good, really. Thanks for checking!

I will fix the other problems in this PR, and for the processes outliving the GUI we will have to make a separate issue.

@ChiCheng45
Copy link
Collaborator

ChiCheng45 commented Mar 6, 2024

Everything seems to work from my testing.

The only thing is that when I run a trajectory conversion, the MDT file are created before its completes. When I terminate the job the files remain. I've created #342 for this issue.

@ChiCheng45 ChiCheng45 merged commit f29beef into protos Mar 6, 2024
54 checks passed
@MBartkowiakSTFC MBartkowiakSTFC deleted the add-qprocess-job-launching branch July 9, 2024 12:47
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.

2 participants