search

neulab / ExplainaBoard

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

fix custom features/analyses loader #601

Closed lyuyangh closed 1 year ago

lyuyangh commented 1 year ago

Overview

We allow custom_features and custom_analyses to be defined for each system. This information is provided as a dict and it may come from the dataset or the system output file provided by the user. In some cases, it is not deserialized so there is a type mismatch. This PR fixes the issue. Please see #596 for details.

Details

References

Blocked by

odashi commented 1 year ago

Test is failing for some reason.

lyuyangh commented 1 year ago

Thanks, @odashi! I fixed the tests.

A couple of pointers for what I changed:

  1. Processor.process() takes deserialized custom_features and custom_analyses (according to the README and the integration tests) so the deserialize logic in Processor._custom_analyses() isn't necessary. That's why I removed it.
  2. MachineTranslationTest.test_custom_features() was failing because dataclasses.asdict() serializes the two fields to dict but, as I mentioned above, Processor.process() expects deserialized objects. I am not sure which fields are actually being used by the processor so I followed the current implementation and included all the fields from FileLoaderMetadata when creating metadata. When #575 is implemented, this code probably needs to be updated.
odashi commented 1 year ago

Yeah, It looks the original code was weird, because {de}serialization is the IO feature and shouldn't be happened in any inner processes (Processor here). The change looks good.

Also, in general we shouldn't use dataclasses.asdict due to its too generic behavior: it expands all nested dataclasses in the object, that possibly brings several breakage into the code.