search

ComPWA / qrules

Rule-based particle reaction problem solver on a quantum number level
https://qrules.rtfd.io
Apache License 2.0
9 stars 4 forks source link

Merge `conservation_rules` and `rule_priorities` attributes #279

Open redeboer opened 1 month ago

redeboer commented 1 month ago

While investigating #272, I started to wonder why EdgeSettings and NodeSettings require conservation_rules as well as rule_priorities. Can't these two just be combined into one attribute of type dict[Rule, int]?

https://github.com/ComPWA/qrules/blob/e60ff895b4cf84be18291b6e4fe0ab516e75a493/src/qrules/solving.py#L44-L50

grayson-helmholz commented 1 month ago

Aside from tests, Edge/NodeSettings is mainly used in settings.create_interaction_settings(...) but which is then heavily employed in the StateTransitionManager. Should this be in its own branch?

redeboer commented 1 month ago

Should this be in its own branch?

Yes as far as possible it is better to keep refactorings isolated and small

redeboer commented 1 month ago

Some idea after discussions: we could write something like:

conservation_rules:  dict[GraphElementRule, RulePriorities]

where RulePriorities is Annotated

grayson-helmholz commented 4 weeks ago

Rewriting get_rules_by_priority() with the new definitions of Edge|NodeSettings causes an infinite-loop in tests/channels/test_y_to_d0_d0bar_pi0_pi0.py

redeboer commented 4 weeks ago

Funny, seems that the rule priorities never did anything from the start 😂 https://github.com/ComPWA/qrules/blob/e60ff895b4cf84be18291b6e4fe0ab516e75a493/src/qrules/solving.py#L539-L541

The keys are x, not type(x), so it always falls back to 1