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

Text classifier runs even when explicitly disabled via command-line #13840

Closed
stephendonner opened this issue Jan 29, 2021 · 5 comments
Closed

Comments

@stephendonner
Copy link

Description

Found while testing brave/brave-core#7533 / #13395; apologies, @tmancey and @moritzhaller if this is a misunderstanding of mine in any way.

Steps to Reproduce

Build ID:

Brave 1.20.93 Chromium: 88.0.4324.96 (Official Build) dev (x86_64)
Revision 68dba2d8a0b149a1d3afac56fa74648032bcf46b-refs/branch-heads/4324@{#1784}
OS macOS Version 11.1 (Build 20C69)

  1. new profile
  2. launch Dev (or Nightly) using --args --enable-logging=stderr --vmodule="*/bat-native-ledger/*"=6,"*/brave_rewards/*"=6,"*/bat-native-ads/*"=6,"*/bat-native-confirmations/*"=6,"*/brave_ads/*"=6,"*/brave_user_model/*"=6 --brave-ads-staging --brave-ads-debug --rewards=staging=true,reconcile-interval=5 --disable-features="TextClassification" --enable-features="EpsilonGreedyBandit"
  3. start using Rewards (and Ads) by invoking and skipping Rewards onboarding
  4. note in the logs:
[16525:775:0129/130425.259234:VERBOSE1:features.cc(59)] Text classification feature is disabled
[16525:775:0129/130425.259284:VERBOSE1:features.cc(61)] Epsilon greedy bandit feature is enabled
[16525:775:0129/130425.259333:VERBOSE1:features.cc(63)] Purchase intent feature is enabled
  1. load google.com
  2. note in the logs:
[16525:775:0129/130425.666700:VERBOSE1:text_classification_resource.cc(56)] Successfully loaded emgmepnebbddgnkhfmhdhmjifkglkamo text classification resource

[16525:775:0129/131009.266567:VERBOSE1:conversions.cc(130)] Checking URL for conversions
[16525:775:0129/131009.267360:VERBOSE1:purchase_intent_processor.cc(97)] No purchase intent matches found for visited URL
[16525:775:0129/131009.267440:VERBOSE1:ads_impl.cc(170)] Search engine pages are not supported for text classification
[16525:775:0129/131009.269385:VERBOSE1:conversions.cc(151)] No conversions found for visited URL

Actual result:

It appears that both:

  1. the text-classification resource is loaded (via log), and
  2. google.com is correctly classified as a search engine

Expected result:

I would expect, given both the command-line arg as well as the corresponding entry "Text classification feature is disabled" iin the logs, for no text-classification resources to be loaded, nor for google.com to be classified (correctly) as a search-engine page, since the feature should be disabled.

Reproduces how often:

100%

Brave version (brave://version info)

Brave 1.20.93 Chromium: 88.0.4324.96 (Official Build) dev (x86_64)
Revision 68dba2d8a0b149a1d3afac56fa74648032bcf46b-refs/branch-heads/4324@{#1784}
OS macOS Version 11.1 (Build 20C69)

Version/Channel Information:

  • Can you reproduce this issue with the current release? n/a
  • Can you reproduce this issue with the beta channel? n/a
  • Can you reproduce this issue with the nightly channel? Yes

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? n/a
  • Does the issue resolve itself when disabling Brave Rewards? n/a
  • Is the issue reproducible on the latest version of Chrome? n/a

Miscellaneous Information:

@stephendonner
Copy link
Author

FWIW, the same thing happens if you disable EpsilonGreedyBandit (by leaving the defaults in place, i.e. not enabling it):

--args --enable-logging=stderr --vmodule="*/bat-native-ledger/*"=6,"*/brave_rewards/*"=6,"*/bat-native-ads/*"=6,"*/bat-native-confirmations/*"=6,"*/brave_ads/*"=6,"*/brave_user_model/*"=6 --brave-ads-staging --brave-ads-debug --rewards=staging=true,reconcile-interval=5 --disable-features="TextClassification"

[16824:775:0129/133601.832643:VERBOSE1:features.cc(59)] Text classification feature is disabled
[16824:775:0129/133601.832673:VERBOSE1:features.cc(61)] Epsilon greedy bandit feature is disabled
[16824:775:0129/133601.832721:VERBOSE1:features.cc(63)] Purchase intent feature is enabled

@tmancey
Copy link
Contributor

tmancey commented Jan 29, 2021

@stephendonner hi, this is intentional so that we build local history when changing between models. @moritzhaller to clarify. Thanks

@moritzhaller
Copy link

@stephendonner yes that is intended as we distinguish between "processors" which are building up local state as the user browses and the matching "models" which use that local state to decide which add to choose. By design the processors will always be active even if its feature is disabled so as to circumvent a cold-start of the feature did we wish to enable it later on. Hope that make sense.

@stephendonner
Copy link
Author

thx for confirming, @tmancey and @moritzhaller; this makes intuitive sense, and is the same (converse) for when bandit is disabled, but still processes/runs, as you mentioned during our EpsilonGreedyBandit call.

I'll put this in my memory banks; besides the early entries:

[16525:775:0129/130425.259234:VERBOSE1:features.cc(59)] Text classification feature is disabled [16525:775:0129/130425.259284:VERBOSE1:features.cc(61)] Epsilon greedy bandit feature is enabled [16525:775:0129/130425.259333:VERBOSE1:features.cc(63)] Purchase intent feature is enabled

would it make sense to somehow annotate in the logs that a particular processor (text classifier, purchase-intent classifier, bandit), even while disabled, is still actively processing, but not being used by the model, or do you see that as superfluous? (I imagine it would add more if/while/else branching/clauses?)

Obviously, feel free to resolve as you see appropriately, and thanks again!

@moritzhaller
Copy link

@stephendonner maybe we could augment the processor logs to indicate that the associated model is enabled/disabled, worth considering. I will close this ticket in the meantime. Thanks!

@tmancey tmancey added this to Ads Jun 10, 2024
@tmancey tmancey moved this to Done in Ads Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

3 participants