neulab / explainaboard_web

MIT License
8 stars 2 forks source link

Upgrade sdk to v0.12.0 #526

Closed lyuyangh closed 1 year ago

lyuyangh commented 1 year ago

I modified the following:

  1. get_loader() -> get_loader_class()
  2. get_processor() -> get_processor_class()
  3. replaced general_to_dict() with serializer.serialize()`
  4. modified MetricResult schema to match the one used by the SDK

I wanted to make as few changes as possible and stick to the current design in this PR. The part that gave me the most trouble is Analysis/utils.tsx which probably contains the most complex code in this repository (it probably needs to be). We can maybe think of ways to refactor it so it is more versatile and add better type annotations so it is easier to do upgrades like this. I can submit a PR to do that sometime in the future.

Testing:

Passed all the items on the checklist. All unit tests are passing.

The confidence intervals for sst2 sample system output are a lot wider. Is this expected?

image

Compatibility

  1. explainaboard_client should be fine. I only changed the interface for POST /systems/analyses which is not used by the client.
  2. No DB migration is required.
PaulCCCCCCH commented 1 year ago

Thanks for the update @lyuyangh !

I have checked the combo analysis and everything looks fine to me except that the matrix axes are wrong. The fix is easy: we can simply go to ComboAnalysisChart.tsx and swap the lines name: analysis.comboFeatures[1] and name: analysis.comboFeatures[0].

The cause: at the time when we merged #457, the order of the feature values returned by the SDK did not match the feature name, so I swapped the order at the frontend. Now the order are fixed in the updated SDK, we need to swap them back.

lyuyangh commented 1 year ago

Thanks, @PaulCCCCCCH ! I have fixed that issue.