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

feat(client): rubrix listener #1507

Merged
merged 8 commits into from May 31, 2022
Merged

feat(client): rubrix listener #1507

merged 8 commits into from May 31, 2022

Conversation

frascuchon
Copy link
Member

@frascuchon frascuchon commented May 19, 2022

The missing component for MLOPs:

Some examples of use:

import time
from typing import List

import rubrix as rb

from rubrix import listener, RBListenerContext


@listener(
    dataset="other-dataset",
    query="status:Validated AND last_updated: [now-5m TO *]",  # we set a time-windows of 5 minutes
    condition=lambda query: 80 <= query.total <= 100,
)
def next_batch_iteration(records: List[rb.TokenClassificationRecord]):
    # Do something with the records
    pass


@listener(
    dataset="sst2_listener",
    query="status:Validated AND (NOT _exists_:metadata.pending_update)",
    condition=lambda query: query.total > 0,
)
def update_external_db(
    records: List[rb.TextClassificationRecord], ctx: RBListenerContext
):
    for record in records:
        # update a mongo db or something else
        record.metadata["pending_update"] = False
    rb.log(records, name=ctx.dataset)  # update received records


@listener(
    dataset="sst2_listener",
    metrics=["F1"],
    condition=lambda query, metrics: metrics.F1.f1_macro < 0.95,
    with_records=False,
    execution_interval_in_seconds=15,
)
def some_dataset_listener(
    ctx: RBListenerContext,
):  # include a cxt: RubrixListenerContext as param instead of name, query,...args
    print(f"The model is degrading the performance: {ctx.metrics.F1}")
    # send an alert
    # or launch a model re-training
    # or buy something in amazon

some_dataset_listener.start()
update_external_db.start()

try:
    while True:
        time.sleep(1)
except KeyboardInterrupt:
    some_dataset_listener.stop()
    update_external_db.stop()

Waiting for your API use feedback before implementing the tests @dvsrepo @dcfidalgo

@codecov
Copy link

codecov bot commented May 19, 2022

Codecov Report

Merging #1507 (45331d3) into master (12937c8) will decrease coverage by 0.07%.
The diff coverage is 92.85%.

@@            Coverage Diff             @@
##           master    #1507      +/-   ##
==========================================
- Coverage   95.52%   95.45%   -0.08%     
==========================================
  Files         126      131       +5     
  Lines        6277     6443     +166     
==========================================
+ Hits         5996     6150     +154     
- Misses        281      293      +12     
Flag Coverage Δ
pytest 95.45% <92.85%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/rubrix/__init__.py 63.15% <0.00%> (-3.51%) ⬇️
src/rubrix/client/apis/searches.py 80.00% <80.00%> (ø)
src/rubrix/client/apis/metrics.py 92.30% <92.30%> (ø)
src/rubrix/listeners/listener.py 95.40% <95.40%> (ø)
src/rubrix/client/api.py 96.90% <100.00%> (+0.11%) ⬆️
src/rubrix/client/sdk/client.py 94.82% <100.00%> (ø)
src/rubrix/listeners/__init__.py 100.00% <100.00%> (ø)
src/rubrix/listeners/models.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 72f854c...45331d3. Read the comment docs.

@frascuchon frascuchon added this to In progress in Release via automation May 24, 2022
@frascuchon frascuchon marked this pull request as ready for review May 25, 2022 21:46
@dcfidalgo
Copy link
Contributor

💯 very nice, looks good to me. One question about the implementation, the MetricsAPI and Searches API are only meant to be used by the listener? If not, maybe we can move them to another PR to keep things nicely separated.

Also, not sure if we could somehow automate the with_records argument, that is if the decorated function has only one parameter, do not pass the records along, if it has two arguments pass them along. Or make it dependent on the argument names ... just an idea.

@frascuchon
Copy link
Member Author

frascuchon commented May 27, 2022

💯 very nice, looks good to me. One question about the implementation, the MetricsAPI and Searches API are only meant to be used by the listener? If not, maybe we can move them to another PR to keep things nicely separated.

For now, the Metrics and Searches APIs are used only by the listener module, but the idea is to move all related operations to these APIs, but this should be applied in different PRs

Also, not sure if we could somehow automate the with_records argument, that is if the decorated function has only one parameter, do not pass the records along, if it has two arguments pass them along. Or make it dependent on the argument names ... just an idea.

Nice feat. I'll investigate a bit. I will include it in this PR if it can be tackled easily.

*Update

The action function can be an arbitrary number of arguments that can be passed on listener.start method. So, including this magic could be complex.

We can start like that and test the different use cases. Maybe, in the future, this param can be removed

@frascuchon frascuchon moved this from In progress to Pending Review in Release May 30, 2022
@frascuchon
Copy link
Member Author

I'll merge this PR and start testing with some use cases.

¡

@frascuchon frascuchon merged commit b1658da into master May 31, 2022
Release automation moved this from Pending Review to Waiting Release May 31, 2022
@frascuchon frascuchon deleted the feat/rubrix-listeners branch May 31, 2022 10:20
@frascuchon frascuchon moved this from Waiting Release to Ready to Release QA in Release Jun 16, 2022
@frascuchon frascuchon moved this from Ready to Release QA to Waiting Release in Release Jun 16, 2022
@frascuchon frascuchon moved this from Waiting Release to Ready to Release QA in Release Jun 16, 2022
@frascuchon frascuchon linked an issue Jul 6, 2022 that may be closed by this pull request
@frascuchon frascuchon removed this from Ready to Release QA in Release Jul 6, 2022
frascuchon added a commit that referenced this pull request Jul 6, 2022
(cherry picked from commit b1658da)

- refactor(listener): support dynamic query parameters (#1586)
(cherry picked from commit 2f54f5b)

- docs(listeners): add to python reference (#1583)
(cherry picked from commit 3e225fe)

- fix: include missing packages for rb.listener reference (#1596)
(cherry picked from commit 870e71d)
frascuchon added a commit that referenced this pull request Jul 7, 2022
(cherry picked from commit b1658da)

- refactor(listener): support dynamic query parameters
(cherry picked from commit 2f54f5b)

- docs(listeners): add to python reference
(cherry picked from commit 3e225fe)

- fix: include missing packages for rb.listener reference
(cherry picked from commit 870e71d)
frascuchon added a commit that referenced this pull request Jul 8, 2022
(cherry picked from commit b1658da)

- refactor(listener): support dynamic query parameters
(cherry picked from commit 2f54f5b)

- docs(listeners): add to python reference
(cherry picked from commit 3e225fe)

- fix: include missing packages for rb.listener reference
(cherry picked from commit 870e71d)
frascuchon added a commit that referenced this pull request Jul 8, 2022
(cherry picked from commit b1658da)

- refactor(listener): support dynamic query parameters
(cherry picked from commit 2f54f5b)

- docs(listeners): add to python reference
(cherry picked from commit 3e225fe)

- fix: include missing packages for rb.listener reference
(cherry picked from commit 870e71d)
frascuchon added a commit that referenced this pull request Jul 8, 2022
(cherry picked from commit b1658da)

- refactor(listener): support dynamic query parameters
(cherry picked from commit 2f54f5b)

- docs(listeners): add to python reference
(cherry picked from commit 3e225fe)

- fix: include missing packages for rb.listener reference
(cherry picked from commit 870e71d)
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.

Rubrix listeners
2 participants