examples/run_summarization_no_trainer: fixed incorrect param to hasattr

[rahular]

What does this PR do?

Fixes a small bug in examples/run_summarization_no_trainer.py which resulted in the script not checkpointing models even if the correct argument was passed from CLI.

Before submitting

• This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
• Did you read the contributor guideline,
  Pull Request section?
• Was this discussed/approved via a Github issue or the forum? Please add a link
  to it if that's the case.
• Did you make sure to update the documentation with your changes? Here are the
  documentation guidelines, and
  here are tips on formatting docstrings.
• Did you write any new necessary tests?

@sgugger @patil-suraj

[HuggingFaceDocBuilderDev]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

[LysandreJik]

Would you like to take a look at this, @muellerzr?

[muellerzr]

@rahular this isn't quite the right solution. What this needs to check here is whether or not the checkpointing steps passed were either "epoch" or a digit. So what this currently does is ignore if epoch was actually passed. With this knowledge in mind do you want to give it another go? I'll be able to give a more detailed review if you're stuck etc in a few hours 😄

[rahular]

Hi @muellerzr, checkpointing_steps = args.checkpointing_steps should already take care of the epoch case, if I'm not mistaken. Here's the flow in comments.

    if hasattr(args, "checkpointing_steps"):                        # check if `args` has `checkpointing_steps`
        checkpointing_steps = args.checkpointing_steps              # stores whatever was passed as a local variable (including `epoch`)  
        if args.checkpointing_steps.isdigit():                      # check if the passed argument is a digit
            checkpointing_steps = int(args.checkpointing_steps)     # overwrite the local variable with the digit after casting
    else:                                                           # local variable is `None` if `args` does not have `checkpointing_steps`
        checkpointing_steps = None

Let me know what you think.

[muellerzr]

@rahular good point! I think in that case something like the following chain may be how we want to do this:

checkpointing_steps = args.checkpointing_steps
if checkpointing_steps is not None and checkpointing_steps.isdigit():
        checkpointing_steps = int(checkpointing_steps)

Since it's always present (it's part of the arg parser) and we really care if it's not None or if it's a digit. Does this make sense to you as well?

(And the default is None anyways)

[rahular]

@muellerzr makes sense! Have simplified it and pushed.

[muellerzr]

Great job!

Likely the rest of the no_trainer scripts will need this update as well. Would you like to tackle this? :) (This can be done in a separate PR)

[muellerzr]

Also make sure to run make style; make quality from the base directory of your fork, to fix the style failure

[rahular]

@muellerzr Sure I can take a look at other trainers as well!

[rahular]

@muellerzr let's close this PR and I will update the other trainers and create a new request.