First PR for Model Parameters Pipeline#1
Open
martinwellman wants to merge 124 commits intomainfrom
Open
Conversation
…d run_model_pipeline
…d run_model_pipeline
Collaborator
|
@martinwellman I was using the code on another project and realized that I missed a mistake. The logistic step should be called logistic-regression and not logistic |
…usly, with rep(0, ...)
…sion' transformation step (to match the actual step name)
…d$data (for the center step example)
martinwellman
requested review from
DougManuel,
rafdoodle and
yulric
and removed request for
DougManuel and
rafdoodle
yulric
approved these changes
Apr 29, 2026
Collaborator
yulric
left a comment
yulric
left a comment
There was a problem hiding this comment.
Looks good! Minor comment that there are spots where stop is called without a custom error being created with .make_error. Is that intentional?
Reminder that this will eventually be moved to the model-parameters package and should use the validate_model_parameter_file function.
Also, did not know about the .make_error standard! Really cool!
Comment on lines
+233
to
+242
| #' \item \code{inaccessible_file}: Raised if a step specification file does | ||
| #' not exist or, if `mod$sandbox_path` is set, is not a descendant of that | ||
| #' directory. | ||
| #' \item \code{invalid_file_format}: Raised if a step specification file | ||
| #' cannot be parsed as a CSV. | ||
| #' \item \code{missing_columns}: Raised when a step specification file is | ||
| #' missing required columns. | ||
| #' \item \code{file_not_added}: Raised indirectly via the step functions if | ||
| #' a file was not successfully added to the model cache; should not occur | ||
| #' in normal use. |
Collaborator
There was a problem hiding this comment.
Does the run_model_pipeline actually raise any of these errors? I would assume they're only raised by the prepare_model_pipeline function?
martinwellman
requested review from
DougManuel and
rafdoodle
and removed request for
DougManuel and
rafdoodle
…l never be raised)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR includes all functionality for performing the following Model Parameters steps:
These are all the steps required by HTNPoRT. Support for additional steps (such as fine-and-gray) can be added in the future. The documentation also includes detailed instructions on how to add support for additional steps (see ADDING_NEW_STEP.md) as well as how to add unit tests for any new steps (see tests/testthat/testdata/step-tests/README.md).
There are unit tests for the utility functions (R/model_parameters_pipeline_utils.R) and for the actual pipeline itself (R/model_parameters_pipeline.R) including all transformation steps. There is also a test that uses all HTNPoRT Model Parameters transformation steps and validation data. The data is passed through the pipeline and compared with the expected predicted risks.