search

NVlabs / FPSci

Aim Training Experiments
Other
70 stars 23 forks source link

Update on general_config.md #423

Closed joohwankimNV closed 1 year ago

joohwankimNV commented 1 year ago

The configuration variable specifying commands seem outdated. I updated it to my knowledge, please double check!

bboudaoud-nv commented 1 year ago

This change seems to be documenting the C++ variable names instead of the Any serialized names we use to get the value from Any. I believe what was previously documented currently is correct.

We can change these names if we think the C++ names are more descriptive, but currently the documentation change will break FPSci correctly serializing these command lists.

bboudaoud-nv commented 1 year ago

@joohwankimNV can you confirm that this change is unnecessary? If so I'll close this PR.

joohwankimNV commented 1 year ago

Yes, I agree this change is unnecessary. We can close this.

Joohwan

From: Ben Boudaoud @.> Sent: Wednesday, April 12, 2023 7:53 AM To: NVlabs/FPSci @.> Cc: Joohwan Kim @.>; Mention @.> Subject: Re: [NVlabs/FPSci] Update on general_config.md (PR #423)

@joohwankimNVhttps://github.com/joohwankimNV can you confirm that this change is unnecessary? If so I'll close this PR.

— Reply to this email directly, view it on GitHubhttps://github.com/NVlabs/FPSci/pull/423#issuecomment-1505419555, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALEHNNVC34FCSP3QII3IZLLXA26XTANCNFSM6AAAAAAWN4T2U4. You are receiving this because you were mentioned.Message ID: @.***>