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

file_refit implementation, control parameters #1151

Closed
rubenarslan opened this issue May 3, 2021 · 4 comments
Closed

file_refit implementation, control parameters #1151

rubenarslan opened this issue May 3, 2021 · 4 comments
Labels
Milestone

Comments

@rubenarslan
Copy link

Hi, file_refit = "on_change" is making my life a lot easier. Three requests though:

  1. It would be great if there was a third "force" option to force a refit, when for some reason or other brms doesn't detect a change (or you just want to warm up your laptop in the winter ;-)).
  2. I would love to be able to set a global option like with brms.backend that I always want to refit on change.
  3. It appears that brms doesn't catch changes to control parameters like adapt_delta and treedepth. If it's easy to add, that would of course be great, but if not, a third option "force" would make things pretty easy as well. I just want to avoid messing with files, as I have to leave my IDE, might confuse files etc.
@paul-buerkner
Copy link
Owner

how about adding file_refit = "always"?

Some global option sounds sensible to me.

Yeah, it could be worthwhile checking for control parameters but then we should also check for number of chains, iterations, etc. and I am not sure this is something we want. @martinmodrak what is your opinion on this?

@martinmodrak
Copy link
Contributor

My two cents:
file_refit = "always" makes sense - especially with a global option. Since you can get "always" behavior by simply omitting/commenting out file the only use case I can see for specifying it by an argument is when you want to automatically save the fit for later processing elsewhere.

Not storing control parameters was intentional primarily because it eases implementation - just storing the control argument is not enough as it would refit even when someone just adds an explicit specification of the deafult value (e.g. adapt_delta = 0.8). The current implementation didn't require any additional information about the fit to be stored, not sure if that would be possible while checking control arguments. Second, I've found it hard to build intuition on which arguments should actually trigger a refit - when model or data changes, this is a very clear cut decision. But if for example someone uses more lax control arguments or fewer iterations than maybe forcing a refit is not sensible. However any such logic would probably not be very transparent to the user. Upon reflection on workflows people use I see why some (maybe most?) users would generally want a refit when changing control parameters, so maybe changing that could be sensible.

@rubenarslan
Copy link
Author

Just a note: commenting out file doesn’t get you the desired behavior, because then you don’t have a file of the fit.

Eg in my example, I was increasing adapt_delta, I don’t need the file with the divergent transitions anymore, so I have to do a round trip to the file system.

That control parameters, chains, etc trigger an automatic refit isn’t super important to me if there is an always option (as I’m going to change parameters knowingly, whereas I might change an aspect of the data without realizing). However, if it’s not a massive chore to implement, I think these changes triggering a refit by default would be better than not doing so. Can still interrupt if it misfires and I guess decreases, explicitly specifying the default, etc are much rarer than increases.

@paul-buerkner
Copy link
Owner

Option 'always' is now implemented along with a global brms.file_refit option. Whether a refit should be triggered by changes of number of iterations etc, I don't know but I think this should be discussed separately.

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

No branches or pull requests

3 participants