diff --git a/src/__test__/redux/actions/pipelines/__snapshots__/runQC.test.js.snap b/src/__test__/redux/actions/pipelines/__snapshots__/runQC.test.js.snap index 6ff3533f00..a982bd666d 100644 --- a/src/__test__/redux/actions/pipelines/__snapshots__/runQC.test.js.snap +++ b/src/__test__/redux/actions/pipelines/__snapshots__/runQC.test.js.snap @@ -24,7 +24,19 @@ exports[`runQC action Dispatches events properly 2`] = ` exports[`runQC action Dispatches status error if loading fails 1`] = `[]`; -exports[`runQC action Runs only the embedding if only changed filter was configureEmbedding 1`] = ` +exports[`runQC action Runs only clustering if only changed filter was clusteringSettings 1`] = ` +[ + { + "payload": {}, + "type": "experimentSettings/discardChangedQCFilters", + }, + { + "type": "cellSets/clusteringUpdating", + }, +] +`; + +exports[`runQC action Runs only the embedding if only changed filter was embeddingSettings 1`] = ` [ { "payload": {}, diff --git a/src/__test__/redux/actions/pipelines/runQC.test.js b/src/__test__/redux/actions/pipelines/runQC.test.js index d0823b2120..40e8fa958e 100644 --- a/src/__test__/redux/actions/pipelines/runQC.test.js +++ b/src/__test__/redux/actions/pipelines/runQC.test.js @@ -13,6 +13,8 @@ import { EXPERIMENT_SETTINGS_DISCARD_CHANGED_QC_FILTERS, } from 'redux/actionTypes/experimentSettings'; +import { CELL_SETS_CLUSTERING_UPDATING } from 'redux/actionTypes/cellSets'; + import { EMBEDDINGS_LOADING } from 'redux/actionTypes/embeddings'; import { runQC } from 'redux/actions/pipeline'; @@ -46,6 +48,7 @@ const initialState = { }, }, }, + cellSets: {}, backendStatus: { [experimentId]: { status: { @@ -102,15 +105,15 @@ describe('runQC action', () => { expect(actions).toMatchSnapshot(); }); - it('Runs only the embedding if only changed filter was configureEmbedding', async () => { + it('Runs only the embedding if only changed filter was embeddingSettings', async () => { fetchMock.resetMocks(); saveProcessingSettings.mockImplementation(() => () => Promise.resolve()); - const onlyConfigureEmbeddingChangedState = _.cloneDeep(initialState); - onlyConfigureEmbeddingChangedState.experimentSettings.processing.meta.changedQCFilters = new Set(['configureEmbedding']); + const onlyEmbeddingSettingsChangedState = _.cloneDeep(initialState); + onlyEmbeddingSettingsChangedState.experimentSettings.processing.meta.changedQCFilters = new Set(['embeddingSettings']); - const store = mockStore(onlyConfigureEmbeddingChangedState); + const store = mockStore(onlyEmbeddingSettingsChangedState); await store.dispatch(runQC(experimentId)); await waitForActions( @@ -125,4 +128,28 @@ describe('runQC action', () => { expect(actions).toMatchSnapshot(); }); + + it('Runs only clustering if only changed filter was clusteringSettings', async () => { + fetchMock.resetMocks(); + + saveProcessingSettings.mockImplementation(() => () => Promise.resolve()); + + const onlyClusteringSettingsChangedState = _.cloneDeep(initialState); + onlyClusteringSettingsChangedState.experimentSettings.processing.meta.changedQCFilters = new Set(['clusteringSettings']); + + const store = mockStore(onlyClusteringSettingsChangedState); + await store.dispatch(runQC(experimentId)); + + await waitForActions( + store, + [EXPERIMENT_SETTINGS_DISCARD_CHANGED_QC_FILTERS, CELL_SETS_CLUSTERING_UPDATING], + ); + + const actions = store.getActions(); + + expect(actions[0].type).toEqual(EXPERIMENT_SETTINGS_DISCARD_CHANGED_QC_FILTERS); + expect(actions[1].type).toEqual(CELL_SETS_CLUSTERING_UPDATING); + + expect(actions).toMatchSnapshot(); + }); }); diff --git a/src/components/data-processing/ConfigureEmbedding/CalculationConfig.jsx b/src/components/data-processing/ConfigureEmbedding/CalculationConfig.jsx index e2c214e70a..6c81897b09 100644 --- a/src/components/data-processing/ConfigureEmbedding/CalculationConfig.jsx +++ b/src/components/data-processing/ConfigureEmbedding/CalculationConfig.jsx @@ -1,5 +1,5 @@ import React, { - useState, useEffect, useCallback, + useState, useEffect, } from 'react'; import _ from 'lodash'; import { useDispatch, useSelector } from 'react-redux'; @@ -9,9 +9,8 @@ import { import PropTypes from 'prop-types'; import { QuestionCircleOutlined } from '@ant-design/icons'; -import { updateFilterSettings, saveProcessingSettings } from 'redux/actions/experimentSettings'; +import { updateFilterSettings } from 'redux/actions/experimentSettings'; -import { runCellSetsClustering } from 'redux/actions/cellSets'; import PreloadContent from '../../PreloadContent'; import SliderWithInput from '../../SliderWithInput'; @@ -32,7 +31,7 @@ const EMBEDD_METHOD_TEXT = 'Reducing the dimensionality does lose some informati + 't-SNE and UMAP are stochastic and very much dependent on choice of parameters (t-SNE even more than UMAP) and can yield very different results in different runs. '; const CalculationConfig = (props) => { - const { experimentId, onConfigChange, disabled } = props; + const { onConfigChange, disabled } = props; const FILTER_UUID = 'configureEmbedding'; const dispatch = useDispatch(); @@ -46,41 +45,19 @@ const CalculationConfig = (props) => { const { umap: umapSettings, tsne: tsneSettings } = data?.embeddingSettings.methodSettings || {}; const { louvain: louvainSettings } = data?.clusteringSettings.methodSettings || {}; - const debouncedClustering = useCallback( - _.debounce((resolution) => { - dispatch(runCellSetsClustering(experimentId, resolution)); - }, 1500), - [], - ); - - const [resolution, setResolution] = useState(null); const [minDistance, setMinDistance] = useState(null); - useEffect(() => { - if (!resolution && louvainSettings) { - setResolution(louvainSettings.resolution); - } - }, [louvainSettings]); - useEffect(() => { if (!minDistance && umapSettings) { setMinDistance(umapSettings.minimumDistance); } }, [umapSettings]); - const dispatchDebounce = useCallback(_.debounce((f) => { - dispatch(f); - }, 1500), []); - const updateSettings = (diff) => { - if (diff.embeddingSettings) { - // If this is an embedding change, indicate to user that their changes are not - // applied until they hit Run. - onConfigChange(); - } else { - // If it's a clustering change, debounce the save process at 1.5s. - dispatchDebounce(saveProcessingSettings(experimentId, FILTER_UUID)); - } + // updates to configure embedding run on worker if they are the only changes + // need to know if change was to embedding or clustering settings + const settingType = Object.keys(diff)[0]; + onConfigChange(settingType); dispatch(updateFilterSettings( FILTER_UUID, @@ -98,8 +75,6 @@ const CalculationConfig = (props) => { }, }, }); - - onConfigChange(); }; const setDistanceMetric = (value) => { updateSettings({ @@ -111,8 +86,6 @@ const CalculationConfig = (props) => { }, }, }); - - onConfigChange(); }; const setLearningRate = (value) => { @@ -125,8 +98,6 @@ const CalculationConfig = (props) => { }, }, }); - - onConfigChange(); }; const setPerplexity = (value) => { updateSettings({ @@ -138,8 +109,6 @@ const CalculationConfig = (props) => { }, }, }); - - onConfigChange(); }; const renderUMAPSettings = () => ( @@ -395,21 +364,14 @@ const CalculationConfig = (props) => { max={10} step={0.1} disabled={disabled} - value={resolution} - onUpdate={(value) => { - if (value === resolution) { return; } - - setResolution(value); - updateSettings({ - clusteringSettings: { - methodSettings: { - louvain: { resolution: value }, - }, + value={louvainSettings.resolution} + onUpdate={(value) => updateSettings({ + clusteringSettings: { + methodSettings: { + louvain: { resolution: value }, }, - }); - - debouncedClustering(value); - }} + }, + })} /> diff --git a/src/pages/experiments/[experimentId]/data-processing/index.jsx b/src/pages/experiments/[experimentId]/data-processing/index.jsx index aed33718d5..2a3b265c79 100644 --- a/src/pages/experiments/[experimentId]/data-processing/index.jsx +++ b/src/pages/experiments/[experimentId]/data-processing/index.jsx @@ -329,7 +329,7 @@ const DataProcessingPage = ({ experimentId, experimentData }) => { onConfigChange(key)} + onConfigChange={(settingType) => onConfigChange(settingType)} stepHadErrors={getStepHadErrors(key)} /> ), @@ -695,6 +695,10 @@ const DataProcessingPage = ({ experimentId, experimentData }) => { Your navigation within Cellenics will be restricted during this time. Do you want to start?

+ ) )} diff --git a/src/redux/actions/pipeline/runQC.js b/src/redux/actions/pipeline/runQC.js index acb3bbb283..a0be83e29e 100644 --- a/src/redux/actions/pipeline/runQC.js +++ b/src/redux/actions/pipeline/runQC.js @@ -9,10 +9,10 @@ import { import { saveProcessingSettings } from 'redux/actions/experimentSettings'; import { loadBackendStatus } from 'redux/actions/backendStatus'; import { loadEmbedding } from 'redux/actions/embedding'; +import { runCellSetsClustering } from 'redux/actions/cellSets'; const runOnlyConfigureEmbedding = async (experimentId, embeddingMethod, dispatch) => { await dispatch(saveProcessingSettings(experimentId, 'configureEmbedding')); - dispatch({ type: EXPERIMENT_SETTINGS_DISCARD_CHANGED_QC_FILTERS, payload: {}, @@ -28,6 +28,22 @@ const runOnlyConfigureEmbedding = async (experimentId, embeddingMethod, dispatch ); }; +const runOnlyClustering = async (experimentId, resolution, dispatch) => { + await dispatch(saveProcessingSettings(experimentId, 'configureEmbedding')); + dispatch({ + type: EXPERIMENT_SETTINGS_DISCARD_CHANGED_QC_FILTERS, + payload: {}, + }); + + // Only configure embedding was changed so we run loadEmbedding + dispatch( + runCellSetsClustering( + experimentId, + resolution, + ), + ); +}; + // Question for review, I thought of implementing this function for all the URLs here // (extracting all the URLs into one single place and using constants to // define which url I am trying to access) @@ -42,12 +58,26 @@ const runQC = (experimentId) => async (dispatch, getState) => { const { processing } = getState().experimentSettings; const { changedQCFilters } = processing.meta; - if (changedQCFilters.size === 1 && changedQCFilters.has('configureEmbedding')) { - runOnlyConfigureEmbedding( - experimentId, - processing.configureEmbedding.embeddingSettings.method, - dispatch, - ); + const embeddingChanged = changedQCFilters.has('embeddingSettings'); + const clusteringChanged = changedQCFilters.has('clusteringSettings'); + const otherChanged = [...changedQCFilters].some((value) => value !== 'embeddingSettings' && value !== 'clusteringSettings'); + + // if only embedding or clustering changed + if (!otherChanged) { + if (embeddingChanged) { + runOnlyConfigureEmbedding( + experimentId, + processing.configureEmbedding.embeddingSettings.method, + dispatch, + ); + } + if (clusteringChanged) { + runOnlyClustering( + experimentId, + processing.configureEmbedding.clusteringSettings.methodSettings.louvain.resolution, + dispatch, + ); + } return; } @@ -57,7 +87,6 @@ const runQC = (experimentId) => async (dispatch, getState) => { const stepConfig = processing[stepKey]; processingConfigDiff[stepKey] = stepConfig; }); - try { // We are only sending the configuration that we know changed // with respect to the one that is already persisted in dynamodb