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

Success markers telemetry #10065

Merged
merged 20 commits into from
Nov 5, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Specified events.json
  • Loading branch information
alwx authored and usc-m committed Nov 3, 2021
commit 2d8b79e882efb92badc08854dbcc76218580907a
58 changes: 58 additions & 0 deletions docs/docs/telemetry/events.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"Model Training",
"Model Testing",
"Model Serving",
"Markers Evaluation",
alwx marked this conversation as resolved.
Show resolved Hide resolved
alwx marked this conversation as resolved.
Show resolved Hide resolved
"Data Handling",
"Miscellaneous"
],
Expand Down Expand Up @@ -422,6 +423,63 @@
"num_synonyms",
"num_regexes"
]
},
"Markers Evaluation Initiated": {
alwx marked this conversation as resolved.
Show resolved Hide resolved
"description": "Triggered when markers evaluation has been initiated.",
alwx marked this conversation as resolved.
Show resolved Hide resolved
"type": "object",
"section": "Markers Evaluation",
alwx marked this conversation as resolved.
Show resolved Hide resolved
"properties": {
"strategy": {
"type": "string",
"description": "Strategy to use when selecting trackers to extract from."
},
"only_extract": {
"type": "boolean",
"description": "Indicates if path to write out statistics hasn't been specified."
},
"seed": {
"type": ["string", "null"],
"description": "The seed to initialise the random number generator for use with the 'sample' strategy."
},
"count": {
"type": ["string", "null"],
"description": "Number of trackers to extract from (for any strategy except 'all')."
}
},
"additionalProperties": false,
"required": [
"count"
]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is saying that count is required, however, it's only required when strategy is first_n or sample. Or am I misunderstanding how this is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that's a mistake, fixed it in the latest commit.

},
"Markers Extracted": {
"description": "Triggered when markers has been extracted.",
alwx marked this conversation as resolved.
Show resolved Hide resolved
"type": "object",
"section": "Markers Evaluation",
alwx marked this conversation as resolved.
Show resolved Hide resolved
"properties": {
"trackers_count": {
"type": "integer",
"description": "Number of processed trackers."
}
},
"additionalProperties": false,
"required": [
"trackers_count"
]
},
"Markers Stats Computed": {
"description": "Triggered when marker stats has been computed.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"description": "Triggered when marker stats has been computed.",
"description": "Triggered when marker stats have been computed.",

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"description": "Triggered when marker stats has been computed.",
"description": "Triggered when marker statistics have been computed.",

Copy link
Contributor

@aeshky aeshky Nov 3, 2021

Choose a reason for hiding this comment

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

Let's use the full word in text, and reserve "stats" for code :)

"type": "object",
"section": "Markers Evaluation",
alwx marked this conversation as resolved.
Show resolved Hide resolved
"properties": {
"trackers_count": {
"type": "integer",
"description": "Number of processed trackers."
}
},
"additionalProperties": false,
"required": [
"trackers_count"
]
}
}
}
7 changes: 2 additions & 5 deletions rasa/cli/evaluate.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,8 @@ def _run_markers(
computed per session will be stored in
'<path-to-stats-folder>/statistics-per-session.csv'.
"""
telemetry.track_markers_evaluation_triggered(
strategy=strategy,
only_extract=stats_file is None,
seed=seed,
count=count,
telemetry.track_markers_evaluation_initiated(
alwx marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

different name suggestion: track_evaluate_markers_initiated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the event is now called "Markers Extraction Initiated" (based on one of your suggestions) so I renamed this function to have a similar name

strategy=strategy, only_extract=stats_file is not None, seed=seed, count=count,
)

domain = Domain.load(domain_path) if domain_path else None
Expand Down
27 changes: 9 additions & 18 deletions rasa/telemetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@
TELEMETRY_VISUALIZATION_STARTED_EVENT = "Story Visualization Started"
TELEMETRY_TEST_CORE_EVENT = "Model Core Tested"
TELEMETRY_TEST_NLU_EVENT = "Model NLU Tested"
TELEMETRY_MARKERS_EVALUATION_TRIGGERED_EVENT = "Markers Evaluation Triggered"
TELEMETRY_MARKERS_EVALUATION_INITIATED_EVENT = "Markers Evaluation Initiated"
TELEMETRY_MARKERS_EXTRACTED_EVENT = "Markers Extracted"
TELEMETRY_MARKERS_STATS_COMPUTED_EVENT = "Markers Stats Computed"

Expand Down Expand Up @@ -1011,14 +1011,11 @@ def track_nlu_model_test(test_data: "TrainingData") -> None:


@ensure_telemetry_enabled
def track_markers_evaluation_triggered(
strategy: Text,
only_extract: bool,
seed: Optional[int],
count: Optional[int],
def track_markers_evaluation_initiated(
strategy: Text, only_extract: bool, seed: Optional[int], count: Optional[int],
) -> None:
_track(
TELEMETRY_MARKERS_EVALUATION_TRIGGERED_EVENT,
TELEMETRY_MARKERS_EVALUATION_INITIATED_EVENT,
{
"strategy": strategy,
"only_extract": only_extract,
Expand All @@ -1029,20 +1026,14 @@ def track_markers_evaluation_triggered(


@ensure_telemetry_enabled
def track_markers_extracted(count: int) -> None:
def track_markers_extracted(trackers_count: int) -> None:
_track(
TELEMETRY_MARKERS_EXTRACTED_EVENT,
{
"count": count
},
TELEMETRY_MARKERS_EXTRACTED_EVENT, {"trackers_count": trackers_count},
)


@ensure_telemetry_enabled
def track_markers_stats_computed(count: int) -> None:
def track_markers_stats_computed(trackers_count: int) -> None:
_track(
TELEMETRY_MARKERS_STATS_COMPUTED_EVENT,
{
"count": count
},
)
TELEMETRY_MARKERS_STATS_COMPUTED_EVENT, {"trackers_count": trackers_count},
)