fix: correct paramset requirement reduction and constraint logpdf fallback#2712
fix: correct paramset requirement reduction and constraint logpdf fallback#2712henryiii wants to merge 1 commit into
Conversation
…lback reduce_paramsets_requirements left leftover empty-set placeholders for modifier attributes the parset does not support, and reported a misleading "expected 9" length error when a user configured an unsupported attribute (because the "undefined" sentinel is a truthy 9-char string). Handle the unsupported-attribute case first: raise a clear "does not use" error when the user explicitly configured it, and otherwise drop the placeholder key. The no-constraint logpdf fallback indexed a 0-d tensor (astensor(0.0)[0]), raising IndexError on both backends; return the scalar directly. Also drop the dead poisson_constraint_combined par_indices attribute. Assisted-by: ClaudeCode:claude-opus-4.8
|
Warning Review limit reached
More reviews will be available in 29 minutes and 56 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
🤖 AI text below 🤖
Part of the code review in #2706.
Summary
parameters/utils.pyreduce_paramsets_requirements: when a parset does not support an attribute (default_v == "undefined"), the reduced dict retained a leftover emptyset()placeholder for that key. It is now dropped. Additionally, configuring an unsupported attribute previously hit the list-length check first and reported a misleadingexpected 9error (the"undefined"sentinel is a truthy 9-character string); the unsupported-attribute case is now handled first and raises a clear"<name> does not use the <attr> attribute."error.constraints.py: the no-constraint fallback ingaussian_constraint_combined.logpdfandpoisson_constraint_combined.logpdfindexed a 0-d tensor (tensorlib.astensor(0.0)[0]), which raisesIndexErroron both the NumPy and JAX backends. It now returns the scalartensorlib.astensor(0.0).self.par_indices = list(range(pdfconfig.npars))attribute inpoisson_constraint_combined.__init__(no references anywhere; the gaussian class has no equivalent).Test plan
tests/test_constraints.py::test_constraint_logpdf_no_constraints: a normal-only model returns scalar0.0from the poisson constraint, and a poisson-only model returns scalar0.0from the gaussian constraint, without raising.tests/test_paramsets.py:reduce_paramsets_requirementsproduces no leftover empty-set values for unsupported attributes, and configuring an unsupported attribute raises the"does not use"error.pytest tests/test_constraints.py tests/test_paramsets.py tests/test_pdf.pypasses (75 passed, 6 backend-skipped);prek -aclean.🤖 Generated with Claude Code