-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
Change the default for istunable
to true.
#2497
base: master
Are you sure you want to change the base?
Conversation
I'm a bit sceptical to this change for two reasons
|
I agree here. Also, the couple of large models I've seen, one can get good fits by tuning just a small number of parameters out of the total, in the order of 1-5% of the total number of params, so the responsibility should be on the user to mark these as tunable. |
In this case should MTKParameters change its behavior? |
That's just not true in general. I can list out a bunch of domains for which that is not true, and I can list out a bunch of domains for which is true. The real issue here is that it is domain dependent. In the ecosystem, it comes out to more like 70% of folks expect things to be tunable by default, 30% other way around. A large portion of that is due to biological and pharmacological domains which have mechanistically-based models in the thousands of parameters. So it is simply false to claim this, and a real solution needs to take the actual landscape into account. What it probably needs is 3-valued logic The next question of course is why the interpretation of
Given that it was previously unused in the ecosystem and we just had a breaking change that essentially no one can install yet (not without a few more bumps around the ecosystem), I think it's fine to get this into the v9 as a late addition, as long as we get it in the next week (@AayushSabharwal). |
There are two slightly different use cases being discussed here. When I added this metadata, "tunable" was intended to indicate that a parameter was free to be chosen by the user of a system in order to tune the system to their liking, such as controller parameters etc (indicated in the docs for tunable). This is different from calibrating a parameter to make the model fit data, what we otherwise call "calibration". If the default is true, the metadata is only useful as an opt-out, while if the default is false, users can opt-in to have some parameters tunable. The functionality is already made to handle an optional default which can be specified when asking for tunable parameters, so the problem is not big in practice. Maybe it would make sense to introduce different metadata to indicate whether or not something can be adjusted to calibrate the model to data, and keep this a separate concept from a parameter being adjusted to meet a performance goal, such a controller parameters. A third idea is to remove the metadata altogether. |
The functional difference is whether they show up as Tunable for autodiff.
It's already a system level property, not a variable level property. Hence the 3 valued logic (which already exists for the most part) |
MTKParameters
considers parameters as tunable unless marked as not tunable from what I understand, so it would be nice for this to agree with whatistunable
reports.