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

use qcng task_config inquiry #87

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

loriab
Copy link
Collaborator

@loriab loriab commented Aug 5, 2023

This isn't a ready-to-merge PR; it's more of a discussion opener. MolSSI/QCEngine#402 pointed out that qcengine-driven optimizations aren't obeying the task_config (formerly local_options) ncores/memory/scratch/etc. user settings passed into qcng.compute_procedure() for user control. He has a little script showing the cores psi4 (the common gradient-generator) is getting, and optking is always giving psi4 single-thread, which I can see is deliberately set by optking to psi4.get_num_threads().

I know that all the ways and means of commencing and routing among psi4/other-gradient-generators/optking/qcengine are complex, and the current setup probably was designed to play nicely with psi4 as top-level in some way. So I wonder if design considerations rule out this PR's suggestion or if there's a way to make it work.

No hurry on this -- just lodging the issue in the right place.

@psi-rking
Copy link
Owner

I would think we could accommodate this. I wonder if @AlexHeide 's string methods coming in are relevant to the solution, as they could involve energies/gradients running in parallel.

@loriab
Copy link
Collaborator Author

loriab commented Aug 6, 2023

thanks, gtk. Yes, threading-wise I can see how strings methods make another layer of complication. The situation I described (qcng.compute_procedure("optking", task_config={"ncores": 8})) technically is ambiguous, too: does optking get 8 or the gradient-supplier get 8. But that seems clear that the user would want the expensive part to get the threads. string methods might want a two-level configuration.

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