transformers icon indicating copy to clipboard operation
transformers copied to clipboard

Refactor Roberta using modular

Open Cyrilvallez opened this issue 1 year ago • 2 comments

What does this PR do?

Refactor Roberta using modular. It also improves the modular converter in the following ways:

  • If any decorators are used on top of classes/methods in the modular_xxx.py, those are used instead of the decorators in the original class
  • If any variables used in the decorators are missing, automatically look for them in the visited files
  • Automatically removes unused variables that may be written as part of the decorators dependencies, but that are not used if the decorators were overwritten
  • Fix the replace_call_to_super which was not behaving correctly if any statements appeared before the super() in the modular_xxx.py

All modified files come from the copied from Roberta.

Cyrilvallez avatar Oct 03 '24 16:10 Cyrilvallez

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

cc @ArthurZucker, could you review the changes please? 🤗

Cyrilvallez avatar Oct 18 '24 15:10 Cyrilvallez

Ok, we now have a better dependency mapping when replacing class nodes in ModularConverterTransformer:

  • we only look for (recursive) dependencies of the new node (after the call to replace_call_to_super). This avoids adding wrong dependencies, and thus removes the need for post-cleaning of classes/functions
  • This is important to not rely on post-cleaning for functions/classes because since imports will be taken care of in the post-cleaning, we could otherwise still keep some of them wrongly (if a "to-be-removed" function/class is the only reason to keep an import)
  • if we have no inheritance from a model-specific class, only check the files for functions imports, and add them and their dependencies if needed

Cyrilvallez avatar Oct 23 '24 14:10 Cyrilvallez

IMO the logic in leave_Class in ModularConverterTransformer is much simpler now, let me know what you think I may be biased 😅🤗

Cyrilvallez avatar Oct 23 '24 14:10 Cyrilvallez