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

[TRIAGE]: Investigate whether we need to maintain a "transientModelConfig" for model-config outputs #4662

Closed
mwdchang opened this issue Sep 4, 2024 · 1 comment · Fixed by #4670 or #4671
Assignees
Labels
task Development task

Comments

@mwdchang
Copy link
Member

mwdchang commented Sep 4, 2024

ModelConfiguration operator state is currently defined as

export interface ModelConfigOperationState extends BaseState {
  transientModelConfig: ModelConfiguration;
  notebookHistory: NotebookHistory[];
  hasCodeRun: boolean;
  datasetModelConfigTaskId: string;
  documentModelConfigTaskId: string;
}

Which is then copied whenever we append an operator output. This data is very heavy, especially transientModelConfig which significantly increase the workflow's size and doesn't really like it is being used.

Since output.state is defined as Partial<ModelConfigOperationState> we don't need save every piece of operator.state into output.state.

For example in simulate, we are selective in what output.state gets:

  emit('append-output', {
    type: SimulateCiemssOperation.outputs[0].type,
    label: nodeOutputLabel(props.node, 'Dataset'),
    value: [datasetResult.id],
    state: {
      currentTimespan: state.currentTimespan,
      numSamples: state.numSamples,
      method: state.method,
      summaryId: summaryResponse?.id,
      forecastId: runId
    },
    isSelected: false
  });

See if we can remove transientModelConfig from output.state.

@mwdchang mwdchang added the task Development task label Sep 4, 2024
@mwdchang mwdchang assigned YohannParis and blanchco and unassigned YohannParis Sep 4, 2024
@blanchco
Copy link
Contributor

blanchco commented Sep 4, 2024

Since we are now using the side panel to select model configs, we no longer need transientModelConfig to be stored in the outputs array state. We can simply populate knobs.transientModelConfig with either the side panel model configuration or state.transientModelConfig. We can apply the same idea to the intervention policy operator as well since it is using the same pattern

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
task Development task
Projects
None yet
3 participants