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

Reduce model_confidence softmax warning in CI Tests #8146

Closed
Imod7 opened this issue Mar 9, 2021 · 14 comments
Closed

Reduce model_confidence softmax warning in CI Tests #8146

Imod7 opened this issue Mar 9, 2021 · 14 comments
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style.

Comments

@Imod7
Copy link
Contributor

Imod7 commented Mar 9, 2021

Python version: Python 3.6 Python 3.7

Operating system (windows, osx, ...): Ubuntu Windows

Issue: This Issue is part of the effort to close the Issue#7738 hence to reduce the amount of warnings that are raised when running all tests in the CI. From a first investigation, it seems that this warning ( UserWarning: model_confidence is set to softmax. It is recommended to try using model_confidence=linear_norm to make it easier to tune fallback thresholds. ) for which the current Issue was opened, had 150 occurrences but in a latest CI run it appears in only 81 tests.

Warning (including full traceback):

2021-03-09T05:38:31.3112981Z rasa/utils/train_utils.py:428: 81 tests with warnings 2021-03-09T05:38:31.3114236Z /home/runner/work/rasa/rasa/rasa/utils/train_utils.py:428: UserWarning: model_confidence is set to softmax. It is recommended to try using model_confidence=linear_norm to make it easier to tune fallback thresholds. 2021-03-09T05:38:31.3115429Z category=UserWarning,

Possible Solutions:

  • I assume changing the model_confidence to linear_norm will eliminate the warning.
  • However, from the moment this description is written until this issue is handled, the recommended value of model_confidence might have changed since this depends on the latest findings from the research team. So maybe rerun the CI tests to see the latest recommendation or one of the tests below.

Command or request that led to error:

  • The warning can be found in any of the following CI tests log files :
    • 1_Run Tests (ubuntu-latest, 3.6, test-non-training).txt
    • 3_Run Tests (ubuntu-latest, 3.7, test-non-training).txt
    • 5_Run Tests (ubuntu-latest, 3.8, test-non-training).txt
    • 9_Run Tests (windows-latest, 3.7, test-non-training).txt
  • Also, the warning can be reproduced when running one of the tests below:

pytest tests/utils/test_train_utils.py::test_confidence_loss_settings pytest tests/utils/test_train_utils.py::test_confidence_similarity_settings pytest tests/core/test_training.py::test_training_script_with_max_history_set

Definition of Done:

  • The model_confidence warning appears in only the cases where it is explicitly tested for softmax.
  • In all the cases that the model_confidence takes a default value then this should be the recommended one.
@Imod7 Imod7 added type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors. area:rasa-oss 🎡 Anything related to the open source Rasa framework type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style. and removed type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors. labels Mar 9, 2021
@Imod7 Imod7 changed the title Eliminate model_confidence softmax warning in CI Tests Reduce model_confidence softmax warning in CI Tests Mar 10, 2021
@iurisevero
Copy link
Contributor

iurisevero commented Mar 25, 2021

Exalate commented:

iurisevero commented:

Hi, I would like to try to solve this issue. Is anyone working on it right now?

@iurisevero
Copy link
Contributor

iurisevero commented Mar 25, 2021

Exalate commented:

iurisevero commented:

So, I was working on it and the first idea I have to solve the warning at test_train_utils.py was to add an if statement to check when model_confidence is equal to soft_max. To avoid code duplication, I defined a function to run the _check_confidence_setting() func and it ended like this:

def test_confidence_loss_settings(
 component_config: Dict[Text, Any], raises_exception: bool
 ):
 component_config[SIMILARITY_TYPE] = INNER
 if component_config[MODEL_CONFIDENCE] == SOFTMAX:
 with pytest.warns(UserWarning):
 run_confidense_loss_settings(component_config, raises_exception)
 else:
 run_confidense_loss_settings(component_config, raises_exception)

def run_confidense_loss_settings(
 component_config: Dict[Text, Any], raises_exception: bool
 ):
 if raises_exception:
 with pytest.raises(InvalidConfigException):
 train_utils._check_confidence_setting(component_config)
 else:
 train_utils._check_confidence_setting(component_config)

I'd like to know, is it running away from the code pattern you follow? (hope no)

Also, I noticed that tests with TEDPolicy was raising the warning as well, because the policy uses sotf_max as model_confidence. The best way to solve that, in my opinion, would be changing the model_confidence of it to linear_norm, but I don't know the impact it could have... Is it a valid approach or the right way would be to catch the warning of these tests as I did in the test_train_utils.py?

@iurisevero
Copy link
Contributor

iurisevero commented Mar 28, 2021

Exalate commented:

iurisevero commented:

I tested to change the model_confidence of TEDPolicy and it broke all policies tests (I presumed something like that would happen...). So I studied a little further about that and found that most of the warning may be happening because of the TEDPolicy, as I said before, and the DIETClassifier. Both of them will be updated at 3.0 version and the softmax as confidence model will be removed, what will make the warnings disappear.

I believe the way to reduce the warnings now is to add the pytest.warns() to catch them when raised, but it may break the tests at 3.0. (Or just filters the warning at pytest.ini, but it doesn't look like an option to me).

@Imod7
Copy link
Contributor Author

Imod7 commented Mar 29, 2021

Exalate commented:

Imod7 commented:

Hey @iurisevero for your comments on this ticket! @twerkmeister any thoughts on this one?

@iurisevero
Copy link
Contributor

iurisevero commented Apr 4, 2021

Exalate commented:

iurisevero commented:

After lots of effort trying to catch all warnings raised, I realized that was too much work for little progress and, maybe, just ignore them at pytest.ini is not a bad idea.

Some files look like it is just impossible to catch the warns, like the tests/test_validator.py, and others have a big amount of warnings, like the tests/core/policies/test_ted_policy.py which raises this warning 60 times.... Ignore them with the

filterwarnings =
ignore:.model_confidence is set to.:UserWarning

felt very valid after some hours

😖

@rgstephens
Copy link
Contributor

rgstephens commented May 14, 2021

Exalate commented:

rgstephens commented:

Why do I get this warning when model_confidence hasn't been set? Seems like the default should be linear_norm or this warning is displayed in error.

@Imod7
Copy link
Contributor Author

Imod7 commented May 14, 2021

Exalate commented:

Imod7 commented:

@iurisevero Thank you so much for your work on this one! @rgstephens Exactly! Based on this ticket I also see linear_norm as a replacement to cosine and inner. However it does not mention that softmax should be disallowed too so it is left in several places like here or here.

  1. Model confidence to be returned during inference. Possible values -
  2. 'softmax' and 'linear_norm'.
    MODEL_CONFIDENCE: SOFTMAX,

and it is also explicitly mentioned that a possible value is softmax. On the other hand there is the warning that recommends linear_norm.

Maybe someone from the research team @dakshvar22 ? can give a more accurate answer on this one. Should the model_confidence be set everywhere to linear_norm or remove/change the warning ?

@dakshvar22
Copy link
Contributor

dakshvar22 commented May 18, 2021

Exalate commented:

dakshvar22 commented:

We cannot set the default value of model_confidence to linear_norm in a minor release of Rasa Open Source. It's a model and API breaking change and hence can only be changed in the next major version which is Rasa 3.0

@iurisevero
Copy link
Contributor

iurisevero commented May 18, 2021

Exalate commented:

iurisevero commented:

As the model_confidence cannot be changed, should the filter warning be implemented to reduce the warnings in CI tests? I think is an alternative until the new major release.

@wochinge
Copy link
Contributor

wochinge commented May 27, 2021

Exalate commented:

wochinge commented:

Should be enough to use pytest.warns to assert the warnings where we expect them and fix the config in the other places, no?

@iurisevero
Copy link
Contributor

iurisevero commented May 27, 2021

Exalate commented:

iurisevero commented:

Well, I tried to do it, and I failed... I couldn't fix all warnings with just pytest.warns, but I have my knowledge limitations, both in python and in Rasa. Maybe someone else can fix it.

I'll look for the changes I made and push them to Github, it may help.

Edit: All work I've done on this issue is in this branch. The first and second commit, to be more specific.

@wochinge
Copy link
Contributor

wochinge commented May 28, 2021

Exalate commented:

wochinge commented:

Could you open up a PR against the rasa repo and then we can help you resolving the remaining things?

@iurisevero
Copy link
Contributor

iurisevero commented May 29, 2021

Exalate commented:

iurisevero commented:

Sure. I'll just undo the revert first, to recover the pytest.warns, and then I open a PR.

I don't remember how far I went in the catches, but I'll try to take a look at it on weekdays.

#8775

@stale
Copy link

stale bot commented Jan 9, 2022

Exalate commented:

stale[bot] commented:

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 9, 2022
@wochinge wochinge removed the stale label Jan 10, 2022
@rasabot-exalate rasabot-exalate added area:rasa-oss and removed type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style. area:rasa-oss 🎡 Anything related to the open source Rasa framework labels Mar 15, 2022 — with Exalate Issue Sync
@m-vdb m-vdb added type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style. area:rasa-oss 🎡 Anything related to the open source Rasa framework and removed type:maintenance labels Mar 16, 2022
@rasabot-exalate rasabot-exalate added area:rasa-oss :ferris wheel: area:rasa-oss 🎡 Anything related to the open source Rasa framework type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style. and removed type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style. area:rasa-oss 🎡 Anything related to the open source Rasa framework type:maintenance labels Mar 17, 2022 — with Exalate Issue Sync
@m-vdb m-vdb closed this as completed Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants