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

Predict methods in OTF should be moved outside of class #14

Closed
stevetorr opened this issue Sep 5, 2019 · 2 comments · Fixed by #61
Closed

Predict methods in OTF should be moved outside of class #14

stevetorr opened this issue Sep 5, 2019 · 2 comments · Fixed by #61
Projects

Comments

@stevetorr
Copy link
Contributor

Hat tip to Lixin and Yu who discovered the following problem (1) after laborious debugging:

1.Apparently parallelization for python fails when multiple processes are operating on the same instance from the same class object.

  1. This also helps to keep the OTF class itself smaller and focused, while freeing up the predict functions to be used for other purposes.

For instance, the module I'm developing of gp_from_aimd has great cause to use the predict functions, and to avoid duplicating code, having them be in a different file allows them to be called without an OTF instance. I currently have implemented this in my development branch, for reviving gp_from_aimd. Lixin has done the same in hers, so one of us will push it eventually.

@jonpvandermause
Copy link
Collaborator

@YuuuuXie @nw13slx Is there still a parallelization bug? I remember noticing an issue with the parallelization in otf a few months ago, but patched it with 5dd377a and haven't noticed any problems since. Seems that the parallel methods as implemented work fine and give a significant speed up.

But yes, the predict on structure methods can be moved, perhaps changed to methods of the gp class.

@stevetorr
Copy link
Contributor Author

To do: Test an OTF job on Odyssey to ensure this works in parallel. Test if moving it to another file breaks anything (moving it outside of a class seems to make it work okay, says Yu).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Version 1.0
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants