Removing unnecessary comp_shape from class NormalMixture

[mohammed052]

This modification assumes that the Mixture class automatically handles the reshaping of component batch dimensions, and there is no longer a need to explicitly specify the comp_shape

Description

Related Issue

• Closes Remove NormalMixture comp_shape kwarg #6986

Checklist

• Checked that the pre-commit linting/style checks pass
• Included tests that prove the fix is effective or that the new feature works
• Added necessary documentation (docstrings and/or example notebooks)
• If you are a pro: each commit corresponds to a relevant logical change

Type of change

• New feature / enhancement
• Bug fix
• Documentation
• Maintenance
• Other (please specify):

---

📚 Documentation preview 📚: https://pymc--7098.org.readthedocs.build/en/7098/

[ricardoV94]

Thanks @mohammed052, running that tests and we can merge if everything passes

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.29%. Comparing base (a033261) to head (aa602ec).

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

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7098   +/-   ##
=======================================
  Coverage   92.29%   92.29%           
=======================================
  Files         100      100           
  Lines       16875    16875           
=======================================
  Hits        15575    15575           
  Misses       1300     1300           

Files	Coverage Δ
pymc/distributions/mixture.py	95.08% <100.00%> (ø)

[ricardoV94]

@mohammed052 you can see here some tests failed because they were still using the removed keyword: https://github.com/pymc-devs/pymc/actions/runs/7492576052/job/20484268751?pr=7098

We need to tweak the tests to not use it, but keep the same behavior otherwise.

[mohammed052]

@ricardoV94 can you please elaborate on what needs to be done next to tweak the tests

[ricardoV94]

@mohammed052 you'll need to investigate yourself, and for that it may be useful to run the tests locally to see what they are doing

[larryshamalama]

Hi @mohammed052, as @ricardoV94 mentioned, you will have to adjust tests in test_mixture.py as comp_shape is still passed as an argument to NormalMixture, which is why the tests are failing. Notice the lines where we have extra_args={"comp_shape": 2}. See snippets below. Once all tests pass, this PR would be ready to merge :)

Let us know if you have any other questions!

pymc/tests/distributions/test_mixture.py

Lines 858 to 884 in 94020c9

	def test_random(self, seeded_test):
	def ref_rand(size, w, mu, sigma):
	component = np.random.choice(w.size, size=size, p=w)
	return np.random.normal(mu[component], sigma[component], size=size)
	continuous_random_tester(
	NormalMixture,
	{
	"w": Simplex(2),
	"mu": Domain([[0.05, 2.5], [-5.0, 1.0]], edges=(None, None)),
	"sigma": Domain([[1, 1], [1.5, 2.0]], edges=(None, None)),
	},
	extra_args={"comp_shape": 2},
	size=1000,
	ref_rand=ref_rand,
	)
	continuous_random_tester(
	NormalMixture,
	{
	"w": Simplex(3),
	"mu": Domain([[-5.0, 1.0, 2.5]], edges=(None, None)),
	"sigma": Domain([[1.5, 2.0, 3.0]], edges=(None, None)),
	},
	extra_args={"comp_shape": 3},
	size=1000,
	ref_rand=ref_rand,
	)

pymc/tests/distributions/test_mixture.py

Lines 893 to 907 in 94020c9

	def test_scalar_components(self):
	nd = 3
	npop = 4
	# [[0, 1, 2, 3], [0, 1, 2, 3], [0, 1, 2, 3]]
	mus = pt.constant(np.full((nd, npop), np.arange(npop)))
	with Model() as model:
	m = NormalMixture(
	"m",
	w=np.ones(npop) / npop,
	mu=mus,
	sigma=1e-5,
	comp_shape=(nd, npop),
	shape=nd,
	)

[mohammed052]

I was facing difficulty earlier in identifying the problem in tests.Thank You @larryshamalama for your input.
I have made required changes in test_mixture.py. @ricardoV94 kindly review them.

[larryshamalama]

Thanks for the changes

I was facing difficulty earlier in identifying the problem in tests.Thank You @larryshamalama for your input. I have made required changes in test_mixture.py. @ricardoV94 kindly review them.

There seems to be a lot more tests in test_mixture.py that has comp_shape as an argument, e.g. test_normal_mixture_nd. To be more time efficient, I can't identify every instance where these tests need modifications; there seem to be more. However, akin to my previous comment, the edits should be very similar.

[mohammed052]

I made a few more changes last week, @ricardoV94 @larryshamalama can you now review the pull request once.

[twiecki]

@mohammed052 you need to run pre-commit

diff --git a/tests/distributions/test_mixture.py b/tests/distributions/test_mixture.py
index ec1baa7..7ce6084 100644
--- a/tests/distributions/test_mixture.py
+++ b/tests/distributions/test_mixture.py
@@ -821,9 +821,7 @@ class TestNormalMixture:
             taus = Gamma("taus", alpha=1, beta=1, shape=comp_shape)
             ws = Dirichlet("ws", np.ones(ncomp), shape=(ncomp,))
             mixture0 = NormalMixture("m", w=ws, mu=mus, tau=taus, shape=nd)
-            obs0 = NormalMixture(
-                "obs", w=ws, mu=mus, tau=taus, observed=observed
-            )
+            obs0 = NormalMixture("obs", w=ws, mu=mus, tau=taus, observed=observed)
 
         with Model() as model1:

[ricardoV94]

I was the one rebasing this PR

[welcome]

]
Congrats on merging your first pull request! 🎉 We here at PyMC are proud of you! 💖 Thank you so much for your contribution 🎁