RBM Code Cleanup

[pradnyeshjoshi]

Description

The following changes are made to facilitate better reuse of existing modules and increase readability,

1. Replaced existing timer with Timer from recommender utils.
2. Separated metrics calculation into tf_evaluation.py.
3. Execution times will not be returned from fit and recommend_k_items functions anymore.
4. Minor changes to variable names.

Related Issues

Checklist:

• I have followed the contribution guidelines and code style for this project.
• I have added tests covering my contributions.
• I have updated the documentation accordingly.
• This PR is being made to staging branch and not to main branch.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[codecov-commenter]

Codecov Report

Merging #1599 (fc3c6dd) into staging (0d23856) will increase coverage by 0.91%.
The diff coverage is 66.66%.

❗ Current head fc3c6dd differs from pull request most recent head 9dec57e. Consider uploading reports for the commit 9dec57e to get more accurate results

@@             Coverage Diff             @@
##           staging    #1599      +/-   ##
===========================================
+ Coverage    58.22%   59.13%   +0.91%     
===========================================
  Files           84       88       +4     
  Lines         8462     8942     +480     
===========================================
+ Hits          4927     5288     +361     
- Misses        3535     3654     +119     

Flag	Coverage Δ
pr-gate	59.13% <66.66%> (+0.91%)	⬆️

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

Impacted Files	Coverage Δ
recommenders/models/rbm/rbm.py	82.24% <66.66%> (+8.58%)	⬆️
recommenders/models/sasrec/model.py	85.19% <0.00%> (ø)
recommenders/models/sasrec/sampler.py	91.66% <0.00%> (ø)
recommenders/models/sasrec/ssept.py	22.35% <0.00%> (ø)
recommenders/models/sasrec/util.py	57.74% <0.00%> (ø)
recommenders/evaluation/spark_evaluation.py	87.05% <0.00%> (+0.44%)	⬆️

---

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 2c3feac...9dec57e. Read the comment docs.

[miguelgfierro]

@pradnyeshjoshi you are getting the same error in the GitHub gpu test as in #1606. Do you know where the error could come from?

[miguelgfierro]

You are doing good Pradnyesh, I added some ideas that are taking following our coding guidelines: https://github.com/microsoft/recommenders/wiki/Coding-Guidelines

As much as you can, try to simplify the code. We follow the Zen of Python:

Beautiful is better than ugly.
Explicit is better than implicit. 
Simple is better than complex.
Complex is better than complicated.

:-)

[miguelgfierro]

There is an error in the ADO pipeline: https://dev.azure.com/best-practices/recommenders/_build/results?buildId=55820&view=logs&j=5f8f9dfe-6536-5030-a755-a0d185c26462&t=9c790e16-9f7c-5445-8be8-0594d5cf6650&l=87

tests/unit/recommenders/models/test_deeprec_model.py .......             [ 14%]
tests/unit/recommenders/models/test_deeprec_utils.py ....                [ 22%]
tests/unit/recommenders/models/test_ncf_singlenode.py ..............     [ 50%]
tests/unit/recommenders/models/test_newsrec_model.py ....                [ 58%]
tests/unit/recommenders/models/test_newsrec_utils.py ....                [ 66%]
tests/unit/recommenders/models/test_rbm.py F..                           [ 72%]
tests/unit/recommenders/models/test_wide_deep_utils.py ...               [ 78%]
tests/unit/recommenders/utils/test_gpu_utils.py ..s....                  [ 92%]
tests/unit/recommenders/utils/test_tf_utils.py ....                      [100%]

=================================== FAILURES ===================================
_______________________________ test_class_init ________________________________

init_rbm = {'display': 20, 'epochs': 10, 'init_stdv': 0.01, 'keep_prob': 0.8, ...}

    @pytest.mark.gpu
    def test_class_init(init_rbm):
        model = RBM(
            hidden_units=init_rbm["n_hidden"],
            training_epoch=init_rbm["epochs"],
            minibatch_size=init_rbm["minibatch"],
            keep_prob=init_rbm["keep_prob"],
            learning_rate=init_rbm["learning_rate"],
            init_stdv=init_rbm["init_stdv"],
            sampling_protocol=init_rbm["sampling_protocol"],




        # learning rate
        assert model.learning_rate == init_rbm["learning_rate"]
        # standard deviation used to initialize the weight matrix from a normal distribution
        assert model.stdv == init_rbm["init_stdv"]
        # sampling protocol used to increase the number of steps in Gibbs sampling
        assert model.sampling_protocol == init_rbm["sampling_protocol"]
        # number of epochs after which the rmse is displayed
>       assert model.display == init_rbm["display"]
E       AttributeError: 'RBM' object has no attribute 'display'

[pradnyeshjoshi]

There is an error in the ADO pipeline: https://dev.azure.com/best-practices/recommenders/_build/results?buildId=55820&view=logs&j=5f8f9dfe-6536-5030-a755-a0d185c26462&t=9c790e16-9f7c-5445-8be8-0594d5cf6650&l=87

tests/unit/recommenders/models/test_deeprec_model.py .......             [ 14%]
tests/unit/recommenders/models/test_deeprec_utils.py ....                [ 22%]
tests/unit/recommenders/models/test_ncf_singlenode.py ..............     [ 50%]
tests/unit/recommenders/models/test_newsrec_model.py ....                [ 58%]
tests/unit/recommenders/models/test_newsrec_utils.py ....                [ 66%]
tests/unit/recommenders/models/test_rbm.py F..                           [ 72%]
tests/unit/recommenders/models/test_wide_deep_utils.py ...               [ 78%]
tests/unit/recommenders/utils/test_gpu_utils.py ..s....                  [ 92%]
tests/unit/recommenders/utils/test_tf_utils.py ....                      [100%]

=================================== FAILURES ===================================
_______________________________ test_class_init ________________________________

init_rbm = {'display': 20, 'epochs': 10, 'init_stdv': 0.01, 'keep_prob': 0.8, ...}

    @pytest.mark.gpu
    def test_class_init(init_rbm):
        model = RBM(
            hidden_units=init_rbm["n_hidden"],
            training_epoch=init_rbm["epochs"],
            minibatch_size=init_rbm["minibatch"],
            keep_prob=init_rbm["keep_prob"],
            learning_rate=init_rbm["learning_rate"],
            init_stdv=init_rbm["init_stdv"],
            sampling_protocol=init_rbm["sampling_protocol"],




        # learning rate
        assert model.learning_rate == init_rbm["learning_rate"]
        # standard deviation used to initialize the weight matrix from a normal distribution
        assert model.stdv == init_rbm["init_stdv"]
        # sampling protocol used to increase the number of steps in Gibbs sampling
        assert model.sampling_protocol == init_rbm["sampling_protocol"]
        # number of epochs after which the rmse is displayed
>       assert model.display == init_rbm["display"]
E       AttributeError: 'RBM' object has no attribute 'display'

@miguelgfierro This error is resolved now. The build-spark check however fails with some spark related errors.

[miguelgfierro]

@miguelgfierro This error is resolved now. The build-spark check however fails with some spark related errors.

great. The spark issue I think it is related to the memory of the test VM, so I wouldn't worry about it

[anargyri]

LGTM

[miguelgfierro]

this is great!