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

feat(marshaling): also marshal during predict #1024

Merged
merged 8 commits into from
May 29, 2024
Merged

Conversation

sebffischer
Copy link
Sponsor Member

@sebffischer sebffischer commented May 8, 2024

Summary:

  • setting unmarshal = FALSE not ensures that all models are in unmarshaled form (resample(), benchmark()). This is just easier to reason about
  • models are now also marshaled for prediction, i.e. for callr encapsulation and parallel prediction (I made a wrong assumption + somehow did not properly test this assumption)
  • the marshaling of models for prediction happens automatically during Learner$predict(), but the model will be restored to its state before prediction.
  • the logic for the marshaling in the worker was refactored and is now much easier to understand
  • The LearnerClassifDebug now got an argument called check_pid that is FALSE by default. If this is set to TRUE, the unmarshal_model() method for model_classif_debug sets the marshal_pid slot of the model. The private $.predict() method of LearnerClassifDebug then checks that the marhsal_pid is equal to Sys.getpid(). This lets us test that the unmarshaling is executed correctly for the prediction.

@sebffischer sebffischer marked this pull request as draft May 8, 2024 08:17
@sebffischer sebffischer marked this pull request as ready for review May 8, 2024 12:43
@sebffischer sebffischer marked this pull request as draft May 8, 2024 12:43
@sebffischer sebffischer changed the title fix(marshaling): also marshal during predict feat(marshaling): also marshal during predict May 13, 2024
@sebffischer sebffischer requested a review from be-marc May 13, 2024 16:04
@sebffischer sebffischer marked this pull request as ready for review May 29, 2024 14:18
@sebffischer sebffischer merged commit 7ad062f into main May 29, 2024
4 checks passed
@sebffischer sebffischer deleted the marshal_predict branch May 29, 2024 14:18
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.

None yet

1 participant