Make Configs Consistent

[StellaAthena]

Currently we have configs with a mix of - and _. For example, we use no-weight-tying but output_layer_init_method. I don't particularly care which way we decide is the correct way, but we should be consistent across the library.

[austinburnett]

Hello! I'm a newcomer here and would be more than happy to work on this issue.

[haileyschoelkopf]

Welcome @austinburnett , thanks for offering to work on this!

So for this issue--actually what's currently done in the repo is that either - or _ in an argument are accepted, and the hyphens are normalized to underscores here: https://github.com/EleutherAI/gpt-neox/blob/b608043be541602170bfcfb8ec9bf85e8a0799e0/megatron/neox_arguments/arguments.py#LL186C13-L196C56 . So we currently are consistent internally on which format is used (underscores are used for all args in NeoXArgs which matches neox_arguments.md).

Would an acceptable solution be to leave things as they are now but just document more clearly in the relevant readme that you can write args with hyphens or underscores in YML files? That's what I'd tend toward personally, if it's acceptable.

[StellaAthena]

So for this issue--actually what's currently done in the repo is that either - or _ in an argument are accepted, and the hyphens are normalized to underscores here: https://github.com/EleutherAI/gpt-neox/blob/b608043be541602170bfcfb8ec9bf85e8a0799e0/megatron/neox_arguments/arguments.py#LL186C13-L196C56 . So we currently are consistent internally on which format is used (underscores are used for all args in NeoXArgs which matches neox_arguments.md).
Would an acceptable solution be to leave things as they are now but just document more clearly in the relevant readme that you can write args with hyphens or underscores in YML files? That's what I'd tend toward personally, if it's acceptable.

Yeah absolutely. Honestly, even just changing all the documentation (incl. demo files) to be consistent would probably be sufficient.

Edit: After thinking about it a bit, I think that the documentation solution is ideal for backwards compatibility. We could add a warning that this will be phased out one day, but honestly it's very minimal work to continue to preserve supporting both so my inclination is to not even bother phasing it out.

[austinburnett]

I believe this would be an acceptable solution as well. I can note in the relevant doc that the '-' args may be deprecated. For the sake of clarity, by consistency you mean modifying everything to have '_' args instead of '-'.

[StellaAthena]

@austinburnett i have a weak preference for making - the encouraged separator, but otherwise yeah you got it completely.

[StellaAthena]

I believe this would be an acceptable solution as well. I can note in the relevant doc that the '-' args may be deprecated. For the sake of clarity, by consistency you mean modifying everything to have '_' args instead of '-'.

I have an extremely weak preference for the other way around, but it really doesn't matter.

[austinburnett]

Alright, sounds good. I'm working on it now. I'll stick with the original plan of converting '-' to '_'. My goal is to develop a regular expression to take care of it.

[austinburnett]

I just opened up a pull request for this issue. I believe I have completed task 1 & 2. However, I'm unsure if task 3 is complete.