Skip to content

Creating and Editing Factorial Experiment through stepper#687

Merged
mfugate1 merged 82 commits intodevfrom
new-design-tab-UI
Jan 31, 2023
Merged

Creating and Editing Factorial Experiment through stepper#687
mfugate1 merged 82 commits intodevfrom
new-design-tab-UI

Conversation

@RidhamShah
Copy link
Copy Markdown
Contributor

No description provided.

Yagnik56 and others added 30 commits December 14, 2022 15:42
@danoswaltCL
Copy link
Copy Markdown
Collaborator

I pushed a change to remove strict validation of context metadata in this branch.

https://carnegielearning.slack.com/archives/CTD5UG8R2/p1674241667763139

const existingPartitions = experimentInfo.partitions;

const levelOrder = {};
existingPartitions.forEach((partition) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this sort strategy will always work? Levels will always "tie" with the other levels in a factor, right? so what guarantees the order of levels within a factor?

We're sending the index as order to the backend for factors, "partitions", and conditions, so can we just add an order property in for levelCombinationsElements and conditionAliases so we don't have to do a workaround like this?

@RidhamShah @ppratikcr7 @VivekFitkariwala ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The levelCombinationElements determines the order of the factors, not the levels. Therefore, in a condition table, all levels will be sorted according to their respective factors. Initially, we considered adding an order for levelCombinationElements, but since the order would be a derived value from the factors and levels, we decided not to include it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, maybe I've got it a little confused then what was showing up out of order exactly, it seems like there still should be a good way to save the order when submitting to backend instead of doing this work in UI. same for conditionAliases table in simple experiment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will look into it.

levels: [tableDataRow.levels],
alias: [tableDataRow.alias],
weight: [this.experimentDesignStepperService.formatDisplayWeight(tableDataRow.weight)],
weight: [this.experimentDesignStepperService.formatDisplayWeight(tableDataRow.weight),Validators.min(0)],
Copy link
Copy Markdown
Collaborator

@danoswaltCL danoswaltCL Jan 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're not hooking into Angular form validation techniques for showing the message, this isn't adding anything to what is already there, it can be taken out.

@danoswaltCL danoswaltCL self-requested a review January 26, 2023 19:47
@ppratikcr7 ppratikcr7 marked this pull request as ready for review January 26, 2023 20:56
@mfugate1 mfugate1 enabled auto-merge (squash) January 26, 2023 20:57
return tableData;
}

convertToPartitionData(factorialExperimentDesignFormData: ExperimentFactorialDesignData) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please change this to something that does not use the word partition.

existingDecisionPoint.site === decisionPoint.site && existingDecisionPoint.target === decisionPoint.target
)?.factors.push(currentFactors)
) {
const partitionData = {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please change this, it still says partition

@mfugate1 mfugate1 merged commit 3e5a568 into dev Jan 31, 2023
@mfugate1 mfugate1 deleted the new-design-tab-UI branch January 31, 2023 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UI design for factorial/partial factorial experiment support

7 participants