We are considering copy pasting the contents of modeling_bart.py into modeling_pegasus.py, modeling_mbart.py and  modeling_marian.py.  and wanted to see what the community thinks.
Currently, here is modeling_pegasus.py:

  
      github.com
  
  
    huggingface/transformers/blob/master/src/transformers/modeling_pegasus.py 3
# coding=utf-8
# Copyright 2020 Google and The HuggingFace Inc. team.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""PyTorch Pegasus model, ported from https://github.com/google-research/pegasus"""


from .configuration_pegasus import PegasusConfig
from .file_utils import add_start_docstrings
from .modeling_bart import BART_START_DOCSTRING, BartForConditionalGeneration


  This file has been truncated. show original

  
  
    
    
  
  


marian and mbart are similar. They do nothing and inherit the complete implementation from bart. They are only their own classes so that AutoConfig/ AutoTokenizer work properly.
Arguments for copy paste

if you want to know how a model works you can read it in one file.
This is how the rest of the repo works and there is no good reason to introduce a stylistic inconsistency here.
we can delete 6 configuration parameters, that would become always True/always False.

        # params needed for mbart
        self.scale_embedding = scale_embedding  # scale factor will be sqrt(d_model) if True
        self.normalize_embedding = normalize_embedding  # True for mbart, False otherwise
        self.normalize_before = normalize_before  # combo of fairseq's encoder_ and decoder_normalize_before
        self.add_final_layer_norm = add_final_layer_norm

        # Params introduced for Marian
        self.add_bias_logits = add_bias_logits
        self.static_position_embeddings = static_position_embeddings


we could also delete 6 corresponding if statements
it might be easier to make one off fixes or modify one model without unintended consequences.
in the survey, 65% of respondents said they wanted more “Self-contained” code. This is what users want!

Arguments against copy paste

we have to add and maintain about 4,500 lines of code. Could slow us down on adding new features/models.
this might make the code harder to maintain. Some bugs/features that require 1 change will require 4 changes.
the implementations could drift, making it more difficult for the same finetuning code to work with multiple models.
Integration tests enforce that you can’t have big unintended consequences when you are refactoring, so the “unintended side effects” point is moot.
It is actually harder to understand what is going on in pegasus/mbart/marian if the code is duplicated. Seeing that pegasus state dict is identical to bart makes reading the code easier.  “It is annoying to have to read so many similar implementations and have no easy way to see the delta.”
Do not repeat yourself/rule of 3 are best practice and there is no good reason for this lib to deviate.
In the survey, people said they wanted better examples/more models faster, having to maintain more code would distract the team from those objectives.
Maintaining equivalent test coverage after copy paste would require adding 111 tests!

Would love to hear other people’s opinions as there is some disagreement within the team.
You can vote here 11 and comment below.
I am probably missing alternative solutions, pros and cons. I would love to hear!