Devs present: @syclik, @wds15, @Stevo15025 and @rok_cesnovar .
TLDR: Green lights for TBB all around. It will be merged for 2.21 if we can also enhance map_rect with TBB. Resource managment will be handled on the Stan level with a possibility for Stan Math helper functions if they can be done reasonably.
We did a summary of what has been a discussion in various places which is:
Any project is fine if they don’t distribute a Stan model executable and are GPLv2
Daniel said that he feels we should include a huge warning that we are including an Apache 2 licensed dependency in the release we do it. We all agreed that it should definitely be visibly noted.
1b) Merging TBB:
Sebastian said that together with the TBB merge he will enable the use of tbbmalloc when STAN_THREADS are on. If STAN_THREADS is not on, there are some worries of a performance degradation on AMD CPUs (5% was the first figure Steve got, but he said it was not a thorough test). Rok will prepare an update script to update script to the latest version (and test it on the PR).
We had a discussion on when do we merge TBB, 2.21 or wait for 2.22. Daniel’s point was to make 2.21 the last “Apache 2 dependency free” release. There was some more discussion there but the final conclusion was if Sebastian can improve map_rect with TBB to enable scheduling (and something else I forgot to write down) we then release it in 2.21. If not and TBB would only be added but not really used outside tbbmalloc we wait for 2.22. Sebastian noted that if we released it for 2.21 that would give us 3 months of testing it in CmdStan and we would see if its really as advertised before we move it to the other interfaces.
1d) Resource managment
Sebastian wanted to discuss how to do resource management with TBB and limiting it with STAN_NUM_THREADS. He mentioned that he felt that it should be done at Stan level, not Stan Math. We were all in agreement that would make the most sense. We then discussed how things would work for Stan Math standalone. Sebastian mentioned that if nothing was explicitly done, the default resource management would be in effect with the Stan Math user having the option of modifying it via TBB. Rok asked if there is an option of adding a helper function in Stan Math to make things easy. That opened some discussion for which the final conclusion was that we should aim for a helper function for resource management in Stan Math if its reasonable to implement it, otherwise at least have some instruction ready on how a user can do it with TBB.
Steve is going to send the TBB book to Rok.
2.) Feature freeze
Everyone is OK with the feature freeze. There was some discussion on how we will handle PRs between the 11th and 18th. Will we restrict merging or tag develop on the 11th? Daniel said that we can go whichever route we want.
- PR policy
Daniel does not like having PR czars that Steve mentioned in the PR reviewing issue. Steve clarified that the czars would be more of a list who to tag if you have no idea who to ask for review. Daniel was OK with that. We then talked about policy at length. Daniel does not like the idea of undocumented functions. We should discuss that some more on the discourse topic. Then put it in the contributing on github after we have a “vote”.
- Performance testing
We had a debate on the required amount of performance testing for PRs, that we should aim for always have faster functions across the board, but acknowledged that there are cases of tradeoffs between speed and readability. Daniel mentioned that we should have more discussion regarding how to do performance testing on the function level. Sebastian noted that the focus should be on the performance impact on the Stan model level rather than function level. No real conclusions were made in this debate, just that we should discuss this more on Discourse.
Sebastian mentioned he is trying out AMICI (https://github.com/ICB-DCM/AMICI) for ODE solving, that it looks fast (I think a 2-fold speedup was mentioned).
If I missed anything or got something wrong let me know.
EDIT: I seem to not be able to spell management :)