Safely end training even if trackers weren't initialized

[Ben-Epstein]

What does this PR do?

If you create an instance of Accelerator() without logging to a tracker, and then call end_training (which would be common in development/debugging), the accelerator shouldn't crash if there aren't trackers. Since self.trackers isn't initialized in the __init__ of the class, this is possible

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?

Tested by running the following

>>> a = Accelerator()
>>> a.end_training()

Made sure this wasn't thrown

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File ".venv/lib/python3.10/site-packages/accelerate/accelerator.py", line 615, in _inner
    return PartialState().on_main_process(function)(*args, **kwargs)
  File ".venv/lib/python3.10/site-packages/accelerate/accelerator.py", line 2499, in end_training
    for tracker in self.trackers:
AttributeError: 'Accelerator' object has no attribute 'trackers'

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

[BenjaminBossan]

I wonder if the better solution wouldn't be to set self.trackers = [] inside of Accelerator.__init__ instead of init_trackers. Within init_trackers, we could call self.trackers.clear() for consistency.

[Ben-Epstein]

I wonder if the better solution wouldn't be to set self.trackers = [] inside of Accelerator.__init__ instead of init_trackers. Within init_trackers, we could call self.trackers.clear() for consistency.

@BenjaminBossan i totally agree, and in fact wrote that first, but I thought there was likely a good reason why that wasn't done in the first place. It's uncommon not to initialize all member variables on __init__ (in fact i'm pretty sure mypy will complain), so I assumed it was purposeful.

Do you think I should go ahead with that?

[BenjaminBossan]

You make a good point that there could be a non-obvious reason. If it's okay to you, we could wait for Zach to comment on this (which may take a week or so).

[HuggingFaceDocBuilderDev]

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

[muellerzr]

Agree with what's being said here. We should set it to [] initially (and honestly shocked we didn't!) and then just keep the original logic :) Would you be willing to do that? Great catch :)

[Ben-Epstein]

Hi @muellerzr updated! I missed your message, sorry about that

[muellerzr]

Thanks for the fix! :)

[BenjaminBossan]

Thanks for updating the PR.

I think now that trackers are initialized in __init__, we can remove the self.trackers = [] inside of init_trackers (here).

Also, we don't need to check getattr(self, "trackers", []) in get_tracker anymore because self.trackers always exists (here).

[muellerzr]

@Ben-Epstein would you like to do Benjamin's comments in this PR? Or can I go with a follow-up PR with it :)

[Ben-Epstein]

Hey thanks @BenjaminBossan -- the first (removing self.trackers in init_trackers) was already removed, but i've cleaned up the getattr and just referenced self.trackers

[BenjaminBossan]

Great, LGTM, thanks for addressing the remaining points.