-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
[ML] Fix sampling probability when switching between automatic and manual #150389
Changes from 7 commits
1a45f6a
7b2b7e9
1d55dd0
b595ef5
d83064d
189a590
a9a8eb2
9c2713f
9dcd987
6f59538
baacd17
340cbad
336e266
1d0e2a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { EuiButton, EuiFlexItem, EuiFormRow, EuiRange, EuiSpacer } from '@elastic/eui'; | ||
import { i18n } from '@kbn/i18n'; | ||
import { isDefined } from '@kbn/ml-is-defined'; | ||
import { FormattedMessage } from '@kbn/i18n-react'; | ||
import React, { useState } from 'react'; | ||
import { | ||
MIN_SAMPLER_PROBABILITY, | ||
RANDOM_SAMPLER_PROBABILITIES, | ||
RANDOM_SAMPLER_STEP, | ||
} from '../../../index_data_visualizer/constants/random_sampler'; | ||
|
||
export const RandomSamplerRangeSlider = ({ | ||
samplingProbability, | ||
setSamplingProbability, | ||
}: { | ||
samplingProbability?: number | null; | ||
setSamplingProbability?: (value: number | null) => void; | ||
}) => { | ||
// Keep track of the input in sampling probability slider when mode is on - manual | ||
// before 'Apply' is clicked | ||
const [samplingProbabilityInput, setSamplingProbabilityInput] = useState(samplingProbability); | ||
const isInvalidSamplingProbabilityInput = | ||
!isDefined(samplingProbabilityInput) || | ||
isNaN(samplingProbabilityInput) || | ||
samplingProbabilityInput <= 0 || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for catching that. Fixed here |
||
samplingProbabilityInput >= 50; | ||
|
||
return ( | ||
<EuiFlexItem grow={true}> | ||
<EuiSpacer size="m" /> | ||
<EuiFormRow | ||
label={i18n.translate( | ||
'xpack.dataVisualizer.randomSamplerSettingsPopUp.randomSamplerPercentageRowLabel', | ||
{ | ||
defaultMessage: 'Sampling percentage', | ||
} | ||
)} | ||
helpText={i18n.translate( | ||
'xpack.dataVisualizer.randomSamplerSettingsPopUp.randomSamplerPercentageRowHelpText', | ||
{ | ||
defaultMessage: 'Choose a value between 0.001% and 50% to randomly sample data.', | ||
} | ||
)} | ||
> | ||
<> | ||
<EuiRange | ||
fullWidth | ||
showTicks | ||
showRange | ||
showInput="inputWithPopover" | ||
min={RANDOM_SAMPLER_STEP} | ||
max={50} | ||
value={(samplingProbabilityInput ?? MIN_SAMPLER_PROBABILITY) * 100} | ||
ticks={RANDOM_SAMPLER_PROBABILITIES.map((d) => ({ | ||
value: d, | ||
label: d === 0.001 || d >= 5 ? `${d}` : '', | ||
}))} | ||
isInvalid={ | ||
!isDefined(samplingProbabilityInput) || | ||
isNaN(samplingProbabilityInput) || | ||
samplingProbabilityInput <= 0 || | ||
samplingProbabilityInput >= 50 | ||
} | ||
onChange={(e) => { | ||
const value = parseFloat((e.target as HTMLInputElement).value); | ||
const prevValue = samplingProbabilityInput ? samplingProbabilityInput * 100 : value; | ||
|
||
if (value > 0 && value <= 1) { | ||
setSamplingProbabilityInput(value / 100); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for catching that. Updated here |
||
} else { | ||
const nextVal = value > prevValue ? Math.ceil(value) : Math.floor(value); | ||
// Because the incremental step is very small (0.0001), | ||
// everytime user clicks the ^ in the numerical input | ||
// we need to make sure it rounds up to the next whole number | ||
setSamplingProbabilityInput(nextVal / 100); | ||
} | ||
}} | ||
step={RANDOM_SAMPLER_STEP} | ||
data-test-subj="dvRandomSamplerProbabilityRange" | ||
append={ | ||
<EuiButton | ||
disabled={isInvalidSamplingProbabilityInput} | ||
onClick={() => { | ||
if (setSamplingProbability && isDefined(samplingProbabilityInput)) { | ||
setSamplingProbability(samplingProbabilityInput); | ||
} | ||
}} | ||
> | ||
<FormattedMessage | ||
id="xpack.dataVisualizer.randomSamplerSettingsPopUp.randomSamplerPercentageApply" | ||
defaultMessage="Apply" | ||
/> | ||
</EuiButton> | ||
} | ||
/> | ||
</> | ||
</EuiFormRow> | ||
</EuiFlexItem> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Suggest to make
ProbabilityUsedMessage
andCalculatingProbMessage
React components on the outer level and not within that component.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated here
1d0e2a6
(#150389)