Skip to content

chore: adding NoneConst and None check #7764

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Hashcode-Ankit
Copy link

@Hashcode-Ankit Hashcode-Ankit commented Apr 27, 2025

Description

Fixes issue: #7762

Related Issue

  • Closes #
  • Related to #

Checklist

Type of change

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

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

Copy link

welcome bot commented Apr 27, 2025

Thank You Banner]
💖 Thanks for opening this pull request! 💖 The PyMC community really appreciates your time and effort to contribute to the project. Please make sure you have read our Contributing Guidelines and filled in our pull request template to the best of your ability.

@@ -289,7 +289,13 @@ def find_measurable_index_mixture(fgraph, node):
# We don't support (non-scalar) integer array indexing as it can pick repeated values,
# but the Mixture logprob assumes all mixture values are independent
if any(
Copy link
Member

Choose a reason for hiding this comment

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

This whole check is a bit of a mess we should think a bit more. It could also be a non-constant slice (see if not)

Copy link
Author

@Hashcode-Ankit Hashcode-Ankit Apr 27, 2025

Choose a reason for hiding this comment

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

@ricardoV94 by this line it could also be a non-constant slice do you mean to say that dynamic slices (non-constant slice) are also need to be skipped?

Also what is your view if we check for tensor variable and only in that case graph node get modified? (solution for mess)

Copy link
Member

Choose a reason for hiding this comment

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

I think this check should be just something like

if any(
            (
                    isinstance(indices, TensorVariable)
                    and indices.dtype.startswith("int")
                    and any(not b for b in indices.type.broadcastable)
                )
            )
            for indices in mixing_indices
        ):

Basically rulling out potentially repeated integer indices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants