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

UCB formula question #126

Open
willfinnigan opened this issue Jul 16, 2023 · 1 comment
Open

UCB formula question #126

willfinnigan opened this issue Jul 16, 2023 · 1 comment

Comments

@willfinnigan
Copy link

Hey Samual and team,

Thanks for this awesome work.

Just a quick question about your UCB formula.

I've seen the UCB1 formula written as wi/ni + c*sqrt(ln(N)/ni).

But in your code you do c*sqrt(2*ln(N)/ni) for the exploration part.

I believe you have C set to 1.4 (ie approximately sqrt(2)). Are you setting C twice? - once hard coded into the UCB formula with the sqrt(2*ln...), and once with the C in the config? Or perhaps I am confused about the UCB formula?

Kind regards,
Will

@SGenheden
Copy link
Contributor

Hello Will,

Thanks for this. The formula implemented is of course from Thakkar et al. Chem Sci (2020) and I don't think that anyone has looked at this equation in 3 years. So thanks for keeping us on our toes :-) At some point in time, I did convince myself that this was correct, but I am unable to do this today. It very much seems like we include the typical value of C twice. I don't necessarily thinks it is anything wrong with using a different value of C than sqrt(2), and we haven't seen much effect of changing C in the past (although I would like to re-do those tests).
So in short: consider the AiZynthFinder to me an atypical implementation of MCTS (it already is :-) in other respects).
I don't see that we will change this anytime soon.

BR
Samuel

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