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

Implement Multi-threading in FairGBM logic #6

Closed
AndreFCruz opened this issue Apr 28, 2022 · 2 comments · May be fixed by #18
Closed

Implement Multi-threading in FairGBM logic #6

AndreFCruz opened this issue Apr 28, 2022 · 2 comments · May be fixed by #18
Labels
enhancement New feature or request XS effort T-shirt effort weighing: XS

Comments

@AndreFCruz
Copy link
Contributor

Summary

Several CPU-bound FairGBM functions are currently single threaded.

TODO:

  • Test if everything works correctly when using the following directive on (most) loops over the data: #pragma omp parallel for schedule(static)
    • The loop at ConstrainedObjectiveFunction::GetConstraintGradientsWRTModelOutput should be the focus, as it is where most CPU time is spent;
    • AFAIK the loop seems to be parallelizable: no variables seem to be changed in the same location (arrays are changed but always at the position of the given row index, therefore no race conditions should occur);
    • If not, adapt the loop such that each thread does not alter variables in common (or use locks when it's impossible to separate the logic);
@AndreFCruz AndreFCruz added the enhancement New feature or request label Apr 28, 2022
@AndreFCruz
Copy link
Contributor Author

AndreFCruz commented May 5, 2022

NOTE: I added comments linking to this issue on the code sections I detected that could use omp multi-threading.
(essentially, it's every time we do a loop over the data; do a CTRL+F for i < num_data_)

@AndreFCruz AndreFCruz added the XS effort T-shirt effort weighing: XS label Jul 20, 2022
@AndreFCruz AndreFCruz added this to the Internal release milestone Jul 20, 2022
@AndreFCruz
Copy link
Contributor Author

UPDATE This issue corresponds to only a few added lines of code on the PR #18; This has already been done and is awaiting other refactors to be merged to master (other refactors will also be done on that PR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request XS effort T-shirt effort weighing: XS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant