Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Freeze not working properly (or almost at all) #57

Open
geniucos opened this issue May 31, 2023 · 1 comment
Open

Freeze not working properly (or almost at all) #57

geniucos opened this issue May 31, 2023 · 1 comment

Comments

@geniucos
Copy link

I was just trying to test out the freeze function and did something like this:

cfg = get_cfg_defaults()
cfg.freeze()
cfg.merge_from_file('input.yaml')

and it turns out merging from a file is let happen although the config node is allegedly frozen. I looked into the code and this makes sense since setattr (which is the only place that actually checks whether the object is immutable) is never called explicitly, but rather lines like this are used instead. Basically in the current, form frozen is only checked for one-time assignments (like cfg.lr=0.1).

There is another behavior which I doubt is intended: judging by the code, both merge_from_list and _merge_a_into_b descent into the tree structure without passing down the immutability information, so one could freeze the global (root) cfg but then be able to modify a grand child of it since the parent hasn't been frozen. Am I missing something? These two points deem freeze() in its current form almost useless.

@Henning742
Copy link

I was reading the comments in the issue section and this might help.
From #56 (comment):

YACS also allows you to enforce immutability so that once you've started your experiment/app and you handle user input, everything is frozen. This is very useful for repeatable experiments because you can log/store the config and you should be able to trust that the parameters weren't modified later on.

Basically, it's to avoid a user accidentally modifying some value in the config later on in the experiment. So if a user is already using merge_from_list, they would know that it's a yacs config and intend to modify it anyway. This might not be what freeze() is designed to prevent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants