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.

Sign up for GitHub

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

Jump to bottom

Configurator Unit Tests #174

Merged
merged 4 commits into from
Jun 18, 2024
Merged

Configurator Unit Tests #174

merged 4 commits into from
Jun 18, 2024

Conversation

Chase-Grajeda
Copy link
Collaborator

Added basic unit tests to the Configurator experimental class, focusing on initialization and correct shape output from the respective configuration filter methods.

The following tests are currently implemented:

  • Configurator initialization with all arguments
  • Configurator initialization with only required arguments
  • Inference variable filter returns correct shape
  • Inference condition filter returns correct shape without summary_outputs
  • Inference condition filter returns correct shape when summary_outputs is present in data
  • Inference condition filter returns correct shape when summary_outputs is present in data and keys
  • Summary variable filter returns correct shape
  • Summary conditions filter returns correct shape
  • Inference condition, summary variable, and summary condition filters return None for missing keys

Feel free to recommend more tests for the suite!

@Chase-Grajeda
First additions
a506e9a 
Added __init__.py for test module.
Added test_configurators.py.
Added basic fixtures and construction tests.
@Chase-Grajeda
Remaining tests
867bca5 
Added remaining unit tests
@Chase-Grajeda Chase-Grajeda added the unit tests A new set of tests needs to be added. label Jun 13, 2024
@Chase-Grajeda Chase-Grajeda self-assigned this Jun 13, 2024
LarsKue
LarsKue requested changes Jun 14, 2024
View reviewed changes
Copy link
Collaborator

@LarsKue LarsKue left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I think this is a solid basis, but we need to be a little bit more thorough with the test parametrization.

tests/test_configurators/test_configurators.py Show resolved Hide resolved
tests/test_configurators/test_configurators.py Outdated

@pytest.fixture()
def test_shape():
return (5,7)
Copy link
Collaborator

Choose a reason for hiding this comment

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

PEP8: space after comma

Copy link
Collaborator

Choose a reason for hiding this comment

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

This fixture should also be parametrized: We want to check for multiple different batch/set sizes and data dims. Ideally, these are minimally small, e.g.:

@pytest.fixture(params=[2, 3])
def batch_size(request):
    return request.param

and same for set_size and num_features

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added to conftest.py and parameterized over [2,3] (see b2922ca)

tests/test_configurators/test_configurators.py Outdated


@pytest.fixture()
def random_data_no_output(test_shape):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't split fixtures like this. Instead, use parametrization:

@pytest.fixture(params=[True, False])
def random_data(request):
    data = ...
    if request.param:
        data["summary_outputs"] = ...
    return data

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Combined repetitive fixtures in conftest.py (see b2922ca)

tests/test_configurators/test_configurators.py Outdated


@pytest.fixture()
def test_params():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also parametrize this fixture. We should have runs where zero/one/multiple variables are used for each input.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Parameterized over [True, False] (see b2922ca)

tests/test_configurators/test_configurators.py Outdated Show resolved Hide resolved
tests/test_configurators/test_configurators.py Outdated
assert config.inference_variables == test_params['inference_variables']
assert config.inference_conditions == []
assert config.summary_variables == []
assert config.summary_conditions == []
Copy link
Collaborator

Choose a reason for hiding this comment

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

same for this one

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Test removed (see b2922ca)

tests/test_configurators/test_configurators.py Outdated
'var2': keras.random.normal(test_shape),
'var3': keras.random.normal(test_shape),
'summary_outputs': keras.random.normal(test_shape),
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We also need to add summary inputs / conditions to this fixture, which should have shape (batch_size, set_size, ...). Note that images should be able to be handled by the configurator as both regular and set input

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added "summary_inputs" and "summary_conditions" to the random_data fixture in conftest.py (see b2922ca). Is this what you were looking for?

tests/test_configurators/test_configurators.py Outdated
def test_inference_vars_filter(random_data, configurator: Configurator, test_shape):
config = configurator
filtered_data = config.configure_inference_variables(random_data)
assert filtered_data.shape == test_shape
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test only works if there is only a single inference_variable, i.e. no concatenation actually happens

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Refactored test to account for multiple tensors (see b2922ca)

tests/test_configurators/test_configurators.py Outdated
filtered_inference_conds = config.configure_inference_conditions(random_data_no_output)
filtered_summary_vars = config.configure_summary_variables(random_data_no_output)
filtered_summary_conds = config.configure_summary_conditions(random_data_no_output)
assert filtered_inference_conds == None
Copy link
Collaborator

Choose a reason for hiding this comment

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

use is None

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Instances of ... == None replaced with ... is None (see b2922ca)

tests/test_configurators/test_configurators.py Outdated

# Test for correct construction of Configurator with all args
def test_configurator_init(test_params, configurator: Configurator):
config = configurator
Copy link
Collaborator

Choose a reason for hiding this comment

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

This renaming has some repeat code smell.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed config = configurator instances and used the respective function arg instead (see b2922ca)

@Chase-Grajeda
Added conftest
a154911 
Separated fixtures and placed them in conftest.py
@codecov-commenter
Copy link

codecov-commenter commented Jun 14, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (streamlined-backend@2f87322). Learn more about missing BASE report.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@                  Coverage Diff                   @@
##             streamlined-backend     #174   +/-   ##
======================================================
  Coverage                       ?   15.24%           
======================================================
  Files                          ?      107           
  Lines                          ?     5181           
  Branches                       ?        0           
======================================================
  Hits                           ?      790           
  Misses                         ?     4391           
  Partials                       ?        0

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Chase-Grajeda
Added requested changes
b2922ca 
Added batch_size, set_size, and num_features parameterizations in conftest.py.
Combined repetitive fixtures in conftest.py.
Combined repetitive tests in test_configurators.py.
Parameterized Configurator initialization in conftest.py.
Parameterized parameter selection in conftest.py.
Removed initialization tests in test_configurators.py.
Added summary_inputs and summary_conditions to parameters.
Changed instances of '==None' to 'is None'.
Removed 'config=Configurator' instances in test_configurators.py.
@LarsKue LarsKue self-requested a review June 18, 2024 13:01
LarsKue
LarsKue approved these changes Jun 18, 2024
View reviewed changes
Copy link
Collaborator

@LarsKue LarsKue left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for the PR

@LarsKue LarsKue marked this pull request as ready for review June 18, 2024 13:15
@LarsKue LarsKue merged commit e2355de into stefanradev93:streamlined-backend Jun 18, 2024
12 checks passed
Chase-Grajeda added a commit to Chase-Grajeda/BayesFlow that referenced this pull request Jun 18, 2024
@Chase-Grajeda
Squashed commit of the following:
83c1f3f 
commit 320f1ae
Author: lars <[email protected]>
Date:   Tue Jun 18 16:23:01 2024 +0200

    fix two moons simulated dtype

commit 27f99cd
Author: lars <[email protected]>
Date:   Tue Jun 18 16:09:45 2024 +0200

    fix data modification for tensorflow compiled mode

commit c8060fc
Merge: 3150d11 e2355de
Author: lars <[email protected]>
Date:   Tue Jun 18 15:35:59 2024 +0200

    Merge remote-tracking branch 'origin/streamlined-backend' into streamlined-backend

commit 3150d11
Author: lars <[email protected]>
Date:   Tue Jun 18 15:35:52 2024 +0200

    add JAX Approximator
    finalize all Approximators

commit e2355de
Author: Chase Grajeda <[email protected]>
Date:   Tue Jun 18 22:15:37 2024 +0900

    Configurator Unit Tests (stefanradev93#174)

    * First additions

    Added __init__.py for test module.
    Added test_configurators.py.
    Added basic fixtures and construction tests.

    * Remaining tests

    Added remaining unit tests

    * Added conftest

    Separated fixtures and placed them in conftest.py

    * Added requested changes

    Added batch_size, set_size, and num_features parameterizations in conftest.py.
    Combined repetitive fixtures in conftest.py.
    Combined repetitive tests in test_configurators.py.
    Parameterized Configurator initialization in conftest.py.
    Parameterized parameter selection in conftest.py.
    Removed initialization tests in test_configurators.py.
    Added summary_inputs and summary_conditions to parameters.
    Changed instances of '==None' to 'is None'.
    Removed 'config=Configurator' instances in test_configurators.py.
LarsKue pushed a commit that referenced this pull request Jun 24, 2024
@Chase-Grajeda
Two moons test fit (#175)
8b47c03 
* Squashed commit of the following:

commit 320f1ae
Author: lars <[email protected]>
Date:   Tue Jun 18 16:23:01 2024 +0200

    fix two moons simulated dtype

commit 27f99cd
Author: lars <[email protected]>
Date:   Tue Jun 18 16:09:45 2024 +0200

    fix data modification for tensorflow compiled mode

commit c8060fc
Merge: 3150d11 e2355de
Author: lars <[email protected]>
Date:   Tue Jun 18 15:35:59 2024 +0200

    Merge remote-tracking branch 'origin/streamlined-backend' into streamlined-backend

commit 3150d11
Author: lars <[email protected]>
Date:   Tue Jun 18 15:35:52 2024 +0200

    add JAX Approximator
    finalize all Approximators

commit e2355de
Author: Chase Grajeda <[email protected]>
Date:   Tue Jun 18 22:15:37 2024 +0900

    Configurator Unit Tests (#174)

    * First additions

    Added __init__.py for test module.
    Added test_configurators.py.
    Added basic fixtures and construction tests.

    * Remaining tests

    Added remaining unit tests

    * Added conftest

    Separated fixtures and placed them in conftest.py

    * Added requested changes

    Added batch_size, set_size, and num_features parameterizations in conftest.py.
    Combined repetitive fixtures in conftest.py.
    Combined repetitive tests in test_configurators.py.
    Parameterized Configurator initialization in conftest.py.
    Parameterized parameter selection in conftest.py.
    Removed initialization tests in test_configurators.py.
    Added summary_inputs and summary_conditions to parameters.
    Changed instances of '==None' to 'is None'.
    Removed 'config=Configurator' instances in test_configurators.py.

* Added loss test

Added test for post-training loss < pre-training loss to test_fit.py::test_fit

* Added vanishing weights test

Added test in test_fit.py::test_fit for vanishing weights

* Added simulator test

Added test to test_fit.py for verifying the simulator produces random and consistent data

* Added MMD test

Added MMD test to test_two_moons.py.
Added MMD method to utils/ops.py.
Added test_dataset to test_two_moons/conftest.py.

* Linting adjustments

Added auto-formatting changes from ruff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LarsKue LarsKue LarsKue approved these changes

@stefanradev93 stefanradev93 Awaiting requested review from stefanradev93

Assignees

@Chase-Grajeda Chase-Grajeda

Labels
unit tests A new set of tests needs to be added.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Chase-Grajeda @codecov-commenter @LarsKue