Expose n_epochs on novel_train and simple_mlp_train - #57
Merged
Conversation
Without one, `run_and_check_output` cannot resolve a value for `--output` and every `*_train` component fails its test with "--output is a required argument".
* Bound novel's training loop with `--n_epochs` rather than `range(100)` * Let `--n_epochs` override the epochs in simple_mlp's bundled model config * Cap both at 2 during `viash test` with `info.test_default` * Skip a step in `test_resources.sh` when its output is already there, and train the model fixtures for 2 epochs
rcannood
added a commit
to benjaminfreyuu/task_predict_modality
that referenced
this pull request
Jul 30, 2026
* Cap the boosting rounds, early stopping patience and nn epochs with `info.test_default`, the way openproblems-bio#57 did for the other two train components * Fold the senkin_tmp fixture into the skip-if-up-to-date structure that came in with openproblems-bio#57
rcannood
added a commit
that referenced
this pull request
Jul 30, 2026
* Add SenKin CITE-seq method (LightGBM + GRU ensemble) * Use senkin_tmp_cite_pred log_normalize and clr_tsvd helpers Replace the inline _lognorm and _clr_tsvd_fitted duplicates in senkin_train with the now-published library functions log_normalize() and clr_tsvd() from senkin_tmp_cite_pred.preprocess. Behaviour is unchanged: log_normalize uses target_sum=1e4 (CP10K + log1p) matching the old _lognorm, and clr_tsvd uses random_state=42 for reproducible components. The previously-kept fitted TSVD object was unused and is dropped. * Densify log_normalize output before array slicing log_normalize() preserves the input sparsity, so on the sparse counts layer it returns a sparse matrix. np.asarray() on a sparse matrix yields a 0-d object array, which crashed at X_lognorm_all[train_mask] with an IndexError. Use the existing _to_dense() helper (and restore float64) so the downstream boolean-mask slicing and concatenation work, matching the previous inline _lognorm behaviour. Verified: viash test src/methods/senkin/senkin_train passes (1/1). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * register senkin, fix its method id, and drop the unused gpu label * Add methods/senkin to run_benchmark's dependencies -- it was in the methods list but not declared, so the generated workflow referenced an undefined variable and the whole run died at startup * Report "senkin" as the method id rather than senkin_predict; the metrics copy uns["method_id"] straight into the score * Drop the gpu label: measured on a T4, senkin runs entirely on CPU (LightGBM's pip wheel is CPU-only and dominates the runtime, and TensorFlow cannot load CUDA in this image) * Give senkin_predict a test_setup and generate its model in test_resources.sh, so viash test has an --input_model to use * rename senkin to senkin_tmp, update doi * Rename the method and both sub-components to `senkin_tmp`, so the name credits both team members * Point the doi at the 2026 competition paper (10.64898/2026.02.24.707614) Co-authored-by: Vladimir Shitov <35199218+VladimirShitov@users.noreply.github.com> * cap senkin_tmp's training settings during viash test * Cap the boosting rounds, early stopping patience and nn epochs with `info.test_default`, the way #57 did for the other two train components * Fold the senkin_tmp fixture into the skip-if-up-to-date structure that came in with #57 * update label * Add test resources --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Robrecht Cannoodt <rcannood@gmail.com> Co-authored-by: Vladimir Shitov <35199218+VladimirShitov@users.noreply.github.com>
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
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.
Depends on #56 -- branched off it, so please merge that one first.
A long-running method should expose whatever bounds its work and cap it for CI,
rather than hardcoding it.
novelhadfor i in range(100)buried inhelper_functions.py, andsimple_mlptook its epoch count from the bundled modelyaml, so neither could be sped up for a test run or tuned by a user.
novel_train:--n_epochs(default 100) bounds the training loopsimple_mlp_train:--n_epochsoverrides the value in the bundled model configwhen given, otherwise the yaml still wins
viash testviainfo.test_default, so the real defaultsare untouched
Checked what the harness actually resolves rather than inferring it from the runtime:
(
simple_mlpwas already only 10 epochs, so its test time is all image build.)Also in
scripts/create_datasets/test_resources.sh:state.yaml, so re-running after a partial failure doesn't redo everything.FORCE=1regenerates.components, not be any good.
The
up_to_datehelper is checked against the cases that matter: