neulab / ExplainaBoard

Interpretable Evaluation for AI Systems
MIT License
361 stars 36 forks source link

AnalysisLevel does not own distinctive information #518

Open odashi opened 2 years ago

odashi commented 2 years ago

Currently, every Processor distinguishes the process according to AnalysisLevel. This struct has name, features, and metric configs. Features and metric configs are specified by the processor itself, and they are only meaningful if the level is used with the same Processor. This means that the owner of these information is Processor, and thus AnalysisLevel is eventually only the key of these values, and not necessary to be formed as a struct. I think it is suitable to (1) remove this struct and (2) return just str as the key of the analysis level names.

RFC: @neubig

neubig commented 2 years ago

I think that's a reasonable alternative design and may be simpler.

My only concern (which I haven't thought about very carefully yet) is whether changing to this design would limit us in our ability to make features more customizable, which is one of our high priority tasks.

odashi commented 2 years ago

I think it is enough to provide a functionality to retrieve a list of features corresponding to given analysis level name.

class MyProcessor(Processor):
  def get_feature_set(name: str) -> List[Feature]:
    if name == "foo":
      return [...]
    elif name == "bar":
      return [...]

The returned features can be modified outside the processor.

odashi commented 2 years ago

I also think that the current AnalysisLevel can become an attribute of the Processor at initialization, or we can implement separate Processors for different AnalysisLevels, to avoid aditional control flow around AnalysisLevels.

neubig commented 2 years ago

The solution in the first of the two above comments seems reasonable to me for now! The latter ones seem like bigger changes that we'd have to think about more carefully