Repository navigation
Reconcile clustering units - #918
Open
Felipedino wants to merge 51 commits into
Open
Felipedino wants to merge 51 commits into
Felipedino wants to merge 51 commits into
Conversation
Refactor ModelJob to delegate task-specific execution to registered TaskExecutor components. Move the supervised execution flow into SupervisedTaskExecutor to keep it compatible with cross-validation changes, and introduce ClusteringTaskExecutor for target-free clustering runs. Based on supervised ModelJob changes by Andrés Salazar.
Extract shared sklearn preprocessing and persistence into SklearnBaseModel. Add SklearnLikeClusterer as the adapter for clustering algorithms and introduce KMeans and DBSCAN wrappers for ClusteringTask.
Adds a converter that fits any registered ClusteringModel, appends a cluster-label column to the dataset, and saves a JSON report with metrics, cluster sizes, profiles, and algorithm specific attributes.
…rage Extended BaseExplorer and ExplorerJob so explorers can declare whether they need the report produced by a converter, which is then loaded and injected at runtime. Converter reports are now stored under notebook/<id>/converters/ instead of datasets/<uuid>/converters/.
Adds ClusteringProfileExplorer, which reads the converter report to summarize a clustering run without requiring column selection. The ClusteringProfileVisualizer renders cluster sizes, evaluation metrics, and the distinctive feature profile of each cluster.
…thm validation Algorithm-specific explorers (HDBSCAN, Agglomerative) raise a descriptive error if the last converter ran a different algorithm. Column selection is validated against the converter scope. Converter report now includes feature_columns and algorithm_key to support both validations.
Clustering trains and evaluates over the entire dataset instead of a train/validation/test partition. Extend the existing Metric.split enum with a FULL value instead of introducing a parallel metrics path, so the runs API, live updates, and model comparison scoring keep working unmodified for tasks with no split.
Clustering sessions train on the full dataset with no validation split, so there is no goal metric to score Optuna trials against. Automatically maximizing an internal metric (Silhouette, Calinski-Harabasz, Davies-Bouldin) also tends to reward degenerate clusterings, since there is no external check on whether the result is meaningful. Hyperparameter optimization is therefore intentionally unsupported for clustering at this stage.
Clustering sessions have no train/validation/test split, so RunResults and LiveMetricsChart now detect this from the session config and adjust accordingly: the split/level selectors are hidden in favor of a single global-evaluation view, and the Predictions tab is hidden (clustering models don't implement predict()) while Explainability and Hyperparameters are left to their existing empty-state handling, matching how any other task without available components already behaves.
Restrict the clustering converter to numeric columns and reject silhouette explorer selections that include columns outside the fitted model's scope. Require an explorer's requires_converter_class to match the most recently finished converter, not just any past match. Always report DBSCAN's noise point count, including zero. Guard the notebook search panel against non-string component metadata.
…to be fixed on units workflow
Two separate faults on the same screen, both of which stopped a session from being created at all. The split strategy was read off taskRequirements without the optional chaining the line above it uses. That state starts as null until its fetch lands, so the component threw on its very first render for every task, not just the ones that need no target. The strategy itself is only ever set by SplitDatasetRows, which a task without splits never renders, so it stayed null and reached the backend as such. ModelSessionParams types the field as a plain str, so the POST came back 422. It now sends the empty string the form was initialised with.
Every clustering algorithm here measures distances, so a column in a wider unit drowns the ones beside it. On a dataset holding scores from 0 to 100 next to hours from 1 to 11, DBSCAN's default eps labels every row noise and Spectral's RBF affinity underflows to an empty graph, which on ten thousand rows took it from four seconds to not finishing. The model session path has no converter step of its own, so the columns are standardised before the model sees them. Both the model and the metrics get the same matrix, so cluster quality is measured in the space the clusters were found in. A notebook that already applied the StandardScaler converter reaches the models by another route and is untouched. The metrics answer None when fewer than two clusters survive, so a degenerate result wrote no rows at all and the run reported success with an empty table and nothing pointing at the parameters behind it. That case now fails with the model, the sample count and the noise count in the message.
The job layer reloads a run through model_cls.load(path), a class level call. Declared as an instance method, the path bound to self and the call failed with a missing argument. The body never used self, so this only aligns it with SklearnLikeModel and SklearnLikeClusterer.
The clustering work shipped without a single test, so nothing in CI touched any of it. The model tests run parametrised over all eight algorithms, so a new one joins the list and inherits the whole contract. The number of groups is not shared between them, so each is paired with the arguments that ask it for three rather than asserting its default. The fixture is sized for FAISS, which wants roughly forty points per centroid before it converges and splits blobs in half below that. The job tests cover the target free branch: that the features are scaled, that the prepare step is the one doing it, and that a run finding fewer than two clusters fails instead of finishing empty.
PrepareDatasetStep conflicted on the same expression from both sides. #856 turned columnsReady from state into a derived value, which is the better shape, but dropped the requiresTarget escape hatch that a task with no output column needs. Kept the derived form with the hatch back inside it, and moved the declaration below requiresTarget: the two are five lines apart in the file and reading it from above would have hit the temporal dead zone on every render. Its new output error is now conditioned on requiresTarget too, so a clustering session no longer paints a red "Required" over a field DivideDatasetColumns does not render for it.
FaissKMeansClustering.train hung a CI runner for six hours. The log of the cancelled job ends on the line before it, right after the six scikit-learn clusterers finished in under a second between them, and the same commit passed on Linux and Windows in four minutes. Rerunning it landed in the same place. FAISS ships its own OpenMP runtime and so does torch, which any process running DashAI has already loaded. Two of them in one process deadlock on macOS the first time FAISS opens a parallel region. Pinning FAISS to a single thread there keeps it out of that region; every other platform keeps the threads the adapter exists for. This is not a test-only problem: a macOS user picking either FAISS model would hang their job the same way, which is why the guard lives in the adapter and every faiss import now goes through it.
There was no timeout, so the deadlock above sat on four macOS runners until GitHub's six hour default cut them off, with no log to read until someone cancelled them by hand. The suite takes four to nine minutes depending on the platform, so thirty is roomy and still turns a hang into a red job with a readable log.
The message was a triple-quoted f-string, so its newline and indentation
reached the user as part of the error. The units branch already reports it as
"Can not prepare Dataset {id} for Task {task}"; this makes both training
paths say exactly that.
Brings the ten develop commits after the #908 sync: RAG retrievers, prompts, embeddings and vectorizers as config objects, diffusion GPU memory modes, releasing VRAM when a generative job fails, and nullable text fields in forms. None of them touches a file the units changed, so the merge has no conflicts.
Clustering was written over the monolithic ModelJob; this brings it onto the units, still training through its own flag. Four files conflicted, three of them rewritten whole by the units, so each was taken from the units side and clustering re-expressed on top: - model_job.py: a task whose REQUIRES_TARGET is False is dispatched before the splits payload and the evaluation strategy are read, since its session stores an empty strategy name. run() skips the prepare, split and build units for it, trains through clustering's helpers, and shares the save tail through SaveModelUnit. - converter_job.py: the converter report is saved from the fitted_converter ApplyConverterUnit publishes, inside the same try as before. - explorer_job.py: the explorer context is built inside RunExplorationUnit, between instantiating the explorer and preparing its dataset, and the config is read only when an explorer asks for a converter report. - base_model.py: the scoring methods stay on BaseModel, as one @Final definition each, with the imports the auto-merge dropped restored. supervised_model.py merged clean with a byte-for-byte copy of develop's scoring block, which would have overridden BaseModel's for every supervised model; it now keeps only the typed train signature.
The existing guard parses BaseModel's own body, so it could not see a subclass redefining compute_metrics. That is what the clustering merge produced: SupervisedModel arrived with a copy of an older scoring block that merged clean and overrode BaseModel's for every supervised model. This parses every module under DashAI/back/models and requires compute_metrics, calculate_metrics, _score_split and _save_metrics to be defined once, on BaseModel. Run against the clustering branch it names SupervisedModel for all four.
Pins what a user or the database can observe of clustering before it is re-expressed over the units: the session as the front stores it, status transitions in order, the Run columns, one FULL/LAST Metric row per metric with the values scored on the standardised features, the saved model and its cluster assignments, the exact text of every error branch, and that a prediction is refused without producing a result. The notebook half pins the Clustering converter's column and report and the explorers' checks. Written and verified against feat/clustering-units untouched; only observable behaviour is asserted, so it has to pass unchanged on the integration branch.
PrepareWithoutTargetUnit is the counterpart of PrepareAndSplitUnit for tasks whose REQUIRES_TARGET is False: it validates the dataset against the task, keeps the input columns and standardises them, and partitions nothing. It publishes the features under their own key, features, a single dataset, rather than reusing x, which is a dict of partitions: a graph validator compares key names, so a reused name with another shape would let a wrong edge through. n_labels comes from the task, so UnsupervisedTask now answers num_labels with None, as a regression task does. ModelJob's target free branch now loads the dataset with LoadDatasetUnit and prepares it with this unit, keeping its old order (dataset, the task's metrics, the features, the model) and reporting every step as a preparation failure, where these errors were reported before. What was left of the helper is split into the metric lookup and the model build, which the next slice replaces with BuildModelUnit. The unit is registered and in EXPECTED_UNITS now, so the contract audit covers it from the first commit that uses it. The scaling tests moved from the job's tests to the unit's.
The target free branch of ModelJob now builds its model with the same BuildModelUnit as every supervised run, validated before the dataset is prepared. That moves the model check ahead of the preparation for this kind of task too, which is a deliberate change: a run with an unknown model and a dataset the task cannot take now reports the model, as a supervised run does, where it used to report the dataset. A test pins the new order; the regression net checks one failure at a time and cannot see it. The job's own model build is gone. Choosing which metrics score the run stays in the job, since it is the job's decision and feeds the scoring unit later.
FitWithoutTargetUnit is the counterpart of FitModelUnit for tasks whose REQUIRES_TARGET is False: it calls model.train(features) and publishes the same instance under the key it came in on, so the unit that saves it sees the object that was trained. It is named after what it does, not after clustering, and saved graphs will persist that name. Its validate() accepts only ClusteringModel, the one family that trains without a target; when another arrives, an UnsupervisedModel base is created and validate() widens to it. ModelJob's target free branch now fits through the unit. What is left of the old helper only scores, as _score_without_target, and reads the labels in a step that still reports as a training failure, as before. The unit is registered and in EXPECTED_UNITS. The job's test for handing the fitted model back became the unit's identity test, and the remaining job tests fit the model before scoring it.
ScoreClustersUnit reads the cluster labels off a fitted model, refuses a
clustering with fewer than two clusters or one per point, and scores the
labels with the metrics named in its config. It returns the scores as
{"full": {...}}, the shape EvaluateModelToArtifactUnit gives its metrics,
and writes nothing, so it is usable as a pipeline node.
Its three failures keep the texts they had: labels that cannot be read are
"Model training failed ...", because to the user the labels are what
training produced; the degenerate clustering keeps its message; a failing
metric is "Metric calculation failed ...". The job wraps each the same way
it did, and a new job-level test pins the label failure with its whole
prefix, which the regression net did not cover.
Scores that are None or not finite are left out, the rule BaseModel applies
to supervised scores; the old path dropped only None. A contract test pins
it, and the regression net, which has no such case, is unchanged.
ModelJob keeps choosing the metrics and writing the FULL/LAST rows with step
0, now as _write_full_metrics; the old scoring helper is gone. The unit is
registered and in EXPECTED_UNITS, and the degenerate cases moved from the
job's tests to the unit's.
An independent review of the merge and slices 2 to 5 found no bugs in the supervised paths of ModelJob, ConverterJob and ExplorerJob, and the three target-free units keep the contract beyond what the audit sees. What it did find: - RunExplorationUnit passes a ValueError from launch_exploration through verbatim for every explorer, not only the clustering ones: six older explorers raise one from there. Kept, since it is what clustering brought over develop, but the traceback now reaches the log and a contract test pins the behaviour. - BaseConverter.changes_row_count() came back through a clean merge after develop had replaced it with CHANGES_ROW_COUNT, and answered False for every converter. Nothing called it; removed. - ConverterJob read the config for every converter. It now reads it only when there is a report to save, as RunExplorationUnit already did. - test_jobs accepted "finished" or "error" for jobs whose dummies the merge rebased; both finish, and that is now what it asserts. - The unknown-metric test of ScoreClustersUnit now calls validate() on an empty context, so it fails if the lookup moves out of it. - The schema the front receives is pinned for the three new units. - Docstrings that contradicted the code: the epoch hook lives on BaseModel, four units write Metric rows indirectly (all gated on run_id), and FitModelUnit requires six keys, not seven.
The claim that the units a clustering run trains through make a graph the engine accepts had been written down but never checked. This validates the graph statically -- seven drawn edges expanding into nine wires, with none of the three new units taking engine-supplied configuration -- and runs it through PipelineJob on the blobs of the clustering regression net. The scores land in the scoring node's artifact and match sklearn on the standardised columns, the model is saved under a pipeline-namespaced directory and recovers the three blobs, and no Run or Metric row is created.
Brings in develop after reconcile/cv-units (#875) and the session preprocessing work (#911, #912). Conflicts: - base_converter.py: both sides added methods at the same place. Kept get_report/build_report (clustering) and infer_output_columns, rows_to_keep and their helpers (develop). - DivideDatasetColumns.jsx: the output picker stays hidden for a task without a target, and takes develop's outputDisabled. - SelectColumnsStep.jsx: develop's flow (inputs from the estimated structure, output fixed by BaseColumnsStep) with the target-free guards on top: no output to wait for, every column an input by default, no output sent. A clustering session never reaches the preprocessing steps: PrepareDatasetStep hides the switch when the task uses no splits. Not marked by git: develop requires every converter to declare COLUMN_OPERATION and to have a case in test_structure_matches_runtime. ClusteringConverter declares "add" (it keeps its scope and appends the label column) and Clustering gets its case.
Felipedino
marked this pull request as draft
October 6, 2026 21:52
cristian-tamblay
marked this pull request as ready for review
October 7, 2026 12:51
…n strategy
Clustering was told apart by REQUIRES_TARGET, checked in five places in
ModelJob alongside the dispatch on the strategy kind, and its sessions were
stored with an empty strategy name and {"splitType": "none"}, which only
passed splits validation because a missing splitter falls back to the default
holdout one.
- Add FullDatasetEvaluationStrategy: KIND "full", scores FULL, offered only
to ClusteringTask.
- ModelJob picks its units from the strategy kind alone: the prepare unit in
_prepare_dataset_and_components and the fitting units in the existing
dispatch. It no longer reads REQUIRES_TARGET, and the preparation result
always has the same keys.
- The session API refuses a strategy its task does not offer, on creation and
on a change of task, which is what lets a full dataset run assume there is
no target. A full dataset session skips splits validation and refuses
session preprocessing, which is fitted per partition.
- Drop SESSION_CONFIG_SCHEMA. The front reads the strategy kind through
useStrategyKind instead of splitType "none"; REQUIRES_TARGET is only read
to hide the output column picker.
- "Error splitting the dataset" becomes "Error preparing the dataset", since
that step now prepares every kind of run, including one that splits nothing.
…et strategy
A strategy that carves nothing takes no splitter, so the session API skipped
splits validation for it and stored whatever payload it was sent, such as a
stale holdout one, which the front then reads back to decide which partitions
the session has. It now accepts only an empty JSON object and answers 422
otherwise, including for the legacy {"splitType": "none"} payload.
This branch has not been deployed
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.
Summary
Brings clustering into develop and runs it on the units in
DashAI/back/units/.Clustering was built on the monolithic
ModelJob, behind a flag that its own comment called temporary. This PR includes the whole feature and replaces that flag with units:Task:
REQUIRES_TARGETandSESSION_CONFIG_SCHEMAonBaseTask, plusSupervisedTask,UnsupervisedTaskandClusteringTask.Models:
SupervisedModelis split out ofBaseModel.ClusteringModelis added with 6 sklearn clusterers and 2 FAISS clusterers.Metrics: Silhouette, Davies-Bouldin and Calinski-Harabasz, with the signature
score(x, labels), andSplitEnum.FULL.Notebook: a
Clusteringconverter that appends a label column and leaves a report, and seven explorers that read that report.Front: the session wizard handles tasks without a target. It shows no output column, no splits, no strategy and no HPO.
Target-free chain:
Clustering trains through this chain, and the same chain runs as a DAG graph.
ModelJobkeeps no clustering helpers except two pieces of glue: choosing the metrics and writing the FULL metric rows.The cut. The units that never read
yare reused unchanged:LoadDatasetUnit,BuildModelUnitandSaveModelUnit. Sibling units are added only whereyis read. The alternative was to passy=Nonethrough the supervised units, which would have overloadedNoneto mean "no target" in five units. It would also have put clustering logic into the units with the most CV coverage. With this cut, no existing unit and no CV regression test changes.The full record of decisions is in
RECONCILING_CLUSTERING.md.Type of Change
faiss-cpudependency.The conflict git did not mark
The clustering branch moved the scoring block from
BaseModelto the newSupervisedModel:_save_metrics,_score_split,calculate_metricsandcompute_metrics. The moved copy was the pre-units version.base_model.pyconflicted, butsupervised_model.pyis a new file, so it merged cleanly.@finalis not enforced at runtime, so whichever side won the conflict,SupervisedModelsilently overrode the units'@finalmethods for every supervised model. As a result:EvaluateModelToArtifactUnittreated "nothing to score" the same as "nothing finite".BaseModelclass.Resolution:
BaseModel: defined once,@final, and with the non-finite filter.SupervisedModelkeeps only the typedtrain.Guard:
tests/back/models/test_metric_methods_defined_once.pyparses all ofDashAI/back/models/**and requires the four methods to be defined only onBaseModel. Run against the clustering branch, it flagsSupervisedModelfor all four.Merging the latest develop surfaced a second case of the same kind. Develop now requires every converter to declare
COLUMN_OPERATIONand to have a case intest_structure_matches_runtime.py.ClusteringConverternow declares"add", because it keeps its scope and appends the label column, andClusteringhas its test case.Deliberate behavior changes
These are measured against clustering as it worked on its own branch, except the explorer change, which also affects explorers that already exist in develop.
BuildModelUnit.validate()now runs before the dataset is prepared, as it does on the supervised path. If a session has both an unpreparable dataset and an unknown model, the error now names the model instead of the dataset. This keeps both paths consistent and validates before doing the expensive preparation.test_model_job_clustering_error_order.pypins the new order; on the clustering branch, the same test fails withCan not prepare Dataset 1 for Task ClusteringTask."Can not prepare Dataset …"is a single line. It was a triple-quoted f-string, so the newline and indentation reached the user literally. It now matches the wording of the units' message insplitter_scope.py. This was committed on the clustering branch before the regression net was written, so the net is still verified against otherwise-untouched clustering.ScoreClustersUnitdrops non-finite scores. ANone, NaN or infinite score is left out, as in supervised scoring. Before, clustering dropped onlyNone. Contract tests intest_score_clusters_unit.pypin this. The net has no non-finite cases, so it is unchanged.ValueErrornow reaches the user verbatim, for every explorer.RunExplorationUnitused to replace it with"Error launching the exploration X.". Now it passes the explorer's own message through, wrapped in aJobError. Clustering brought this rule in, but six existing explorers also raiseValueErrorfromlaunch_exploration: class overlap, autocorrelation, seasonal decomposition, box plot, time index audit and time series plot. That is why the rule applies to all of them. These messages describe a problem with the user's data and are already written for the user. The traceback is still logged, andtest_an_explorer_complaint_reaches_the_user_verbatim_and_the_logpins the behavior.BaseConverter.changes_row_count()is removed. It came back from the clustering branch without a conflict, although develop had already replaced it with theCHANGES_ROW_COUNTattribute. It returnedFalsefor every converter, which contradicts that attribute, and nothing called it. Because it was removed before reaching develop, it does not appear in the final diff.Changes (by file)
Review first
DashAI/back/job/model_job.py: the dispatch onREQUIRES_TARGETand the target-free branch.evaluation_strategynever reaches the lookup.DashAI/back/units/prepare_without_target_unit.py,fit_without_target_unit.pyandscore_clusters_unit.py, with their tests intests/back/units/.RUNTIME_PARAMS, so all three can be used as graph nodes.features, instead of reusingx.xis a partitionedDatasetDict, and the DAG validator compares only key names.DashAI/back/models/base_model.py,supervised_model.pyand the new guard test (see above).DashAI/back/units/run_exploration_unit.py: builds the explorer's report context, readingdi["config"]only when an explorer asks for a report, and handles theValueErrorchange.DashAI/back/job/converter_job.py: saves the fitted converter's report, inside the same try block.DashAI/back/converters/base_converter.py:get_report/build_report.converters/category/clustering.py:COLUMN_OPERATION = "add".DashAI/front/src/components/models/modelSession/SelectColumnsStep.jsx: develop's preprocessing flow with the target-free guards on top. A clustering session never reaches the preprocessing steps, becausePrepareDatasetStephides the switch when the task uses no splits.Mechanical or additive
BaseModeltoSupervisedModel, a one-line change each.SupervisedTask. Theirprepare_for_taskoverrides are removed. Each one only calledsuper(), and the extra loop inClassificationTaskwas a no-op.initial_components.pyregisters the new components.test_units_api.pyadds the three units toEXPECTED_UNITSand pins their schemas.clustering.png,uv.lock.Testing
test_model_job_clustering_net.py(20 tests) pins:step=0per metric, with values checked against sklearn;test_clustering_notebook_net.py(11 tests) pins the converter's label column and report, and the explorers' output and rejections.test_clustering_as_a_graph.py(6 tests) validates the target-free chain as a DAG and runs it withPipelineJob. All nodes finish. The metrics match sklearn, the saved model separates the three blobs, and noRunorMetricrow is written.test_metric_methods_defined_once.py(5 tests).tests/back/test_main.py::test_app_frontdeselected;EXPECTED_UNITS: 37;src/components/modelspass (46).Notes
Out of scope. These are known issues that this PR records but does not fix:
transform. It appends the training labels without looking at the rows it receives, and it detects "unseen rows" only by length. With row scope, it fits on a subset and transforms everything. It also appears unfiltered in the session preprocessing palette, and the backend does not reject session preprocessing for a task without a target when the session is created through the API.POST /run/does not check that the model is compatible with the task. This affects every task: for example, it also accepts a classifier in a regression session. On the target-free path,FitWithoutTargetUnit.validate()catches it today.fitted_converterthrough the context.BaseTaskhas noABCMeta, so none of its abstract methods has ever been enforced.PredictJob.runstill raises anHTTPException. Clustering makes its 422 reachable through the API.evaluation_strategyvalidation, remaining multi-line messages) are listed in §9 of the reconciliation document.