OpenPipe/ART

Bug: art.rewards.ruler function and module collision

Opened this issue · 3 comments

Art version: 0.4.4
Python: 3.12.11

the function art.rewards.ruler.ruler() and the module itself art.rewards.ruler have the same name.
Since in the art.rewards.__init__ you import the function art.rewards.ruler.ruler() as "ruler", a collision occurs between the module name and the function.

To demonstrate:

(1)

import art.rewards.ruler as art_ruler
print(art_ruler.DEFAULT_RUBRIC)

(2)

from art.rewards import ruler as art_ruler
print(art_ruler.DEFAULT_RUBRIC)

(3)

import art.rewards.ruler
print(art.rewards.ruler.DEFAULT_RUBRIC)

All Resulting in the error:

AttributeError: 'function' object has no attribute 'DEFAULT_RUBRIC'

Basically, I dont even think it's possible to somehow import DEFAULT_RUBRIC, or anything which is defined inside the
art.rewards.ruler module while not being imported in art.rewards.__init__.

fixing it will be a breaking change either way I believe, so I would suggest renaming the function art.rewards.ruler.ruler() to something else, as most likely those who use RULER probably use it from the ruler_score_group() function.

Hi @giladfrid009,
Do you find it useful to be able to import DEFAULT_RUBRIC? It was originally meant for internal use, which is why we didn’t expose it.
I’m not sure we’ll be adding enough new utilities to that file to justify a refactor.

Im trying to create a config class which holds RULER config (the idea is to serialize it). ideally the default value of the rubric in that config would've been DEFAULT_RUBRIC. I can solve this issue in other ways, but they are not ideal.

An easy alternative which solves at least my issue and is not a breaking change - is to allow a None value for the rubric parameter in the functions ruler() and ruler_score_group() - in case of None a default rubric would be used, e.g:

def ruler(...)
    if rubric is None:
        rubric = DEFAULT_RUBRIC

But i believe the issue is wider, as any method / class declared in art.rewards.ruler and not explicitly mentioned in art.rewards.__init__ can't be imported, which is not ideal.

Hey @Kovbo, any update?
If no changes are planned w.r.t this issue feel free to close this.