openforcefield / smarty

Chemical perception tree automated exploration tool.
http://openforcefield.org
MIT License
19 stars 8 forks source link

Updating SMARTY decorators #259

Closed camizanette closed 7 years ago

camizanette commented 7 years ago

Just an update of the decorators used on SMARTY. I added the rings decorators:

% Size of smallest ring
r3
r4
r5
r6
% Number of rings
R0
R1
R2
R3
R4
R

@bannanc , could you check if we need anything else there?

bannanc commented 7 years ago

So 'R' isn't number of rings, it's number of ring bonds. So R1 never exists so you should probably remove it. I mis understood that also when I made the decorator/odds files for smirky.

camizanette commented 7 years ago

So we should probably also update the SMIRKY decorators file? I just did a copy and paste of them.

camizanette commented 7 years ago

@bannanc could you check this again?

bannanc commented 7 years ago

Sorry, I didn't look at this earlier. @camizanette which file do you actually use for smarty simulations? For the record, R (has ring bonds) and !R0 (doesn't have 0 ring bonds) mean the same thing so you're giving that decorate double probability by including both.

bannanc commented 7 years ago

So you know @camizanette the odds files in smarty/data I consider examples, they don't reflect the files I'm using for simulations where I weight the decorators unevenly.

camizanette commented 7 years ago

@bannanc I have been using new-decorators.smarts file to run on SMARTY. I am not including the "!" decorators there. Even though you are not using the files for your tests, I think they should have the correct decorators names, so I changed them accordingly.

bannanc commented 7 years ago

Oh OK, I misread which file you were changing, I agree that they should have the proper label here. I'll approve and merge this.