[random] geometric_distribution backwards compatibility
AMDG Over the last year I've been (slowly) updating Boost.Random to match the new standard. Unfortunately I just hit a problem that I don't have a good solution for. In the standard, the geometric_distribution is defined as $p(i) = p(1-p)^{i}$, with i >= 0. In the current Boost.Random code it is $p(i) = (1-p) p^{i-1}$, with i >= 1. The definition in the standard makes more sense, but I don't want to introduce a breaking change. I can see two options here: * Leave the implementation alone and put a big warning in the documentation saying that the behavior doesn't match the standard. * Since I'm also moving things into namespace boost::random, I can use the new definition and leave a version with the old behavior in namespace boost instead of having a using declaration. I'd still have to add a warning that boost::geometric_distribution isn't the same as boost::random::geometric_distribution. Neither one is particularly appealing. Any thoughts? Better ideas? In Christ, Steven Watanabe
On Sun, 23 Jan 2011, Steven Watanabe wrote:
AMDG
Over the last year I've been (slowly) updating Boost.Random to match the new standard. Unfortunately I just hit a problem that I don't have a good solution for. In the standard, the geometric_distribution is defined as $p(i) = p(1-p)^{i}$, with i >= 0. In the current Boost.Random code it is $p(i) = (1-p) p^{i-1}$, with i >= 1. The definition in the standard makes more sense, but I don't want to introduce a breaking change. I can see two options here:
* Leave the implementation alone and put a big warning in the documentation saying that the behavior doesn't match the standard. * Since I'm also moving things into namespace boost::random, I can use the new definition and leave a version with the old behavior in namespace boost instead of having a using declaration. I'd still have to add a warning that boost::geometric_distribution isn't the same as boost::random::geometric_distribution.
Neither one is particularly appealing. Any thoughts? Better ideas?
I noticed that discrepancy between Boost.Random and usual definitions of geometric_distribution as well, but forgot to report it; I would prefer to have the standard definition rather than the current one. I support your second option, as long as people don't have both "using namespace boost;" and "using namespace boost::random;" in the same code. Since you state that you will be moving things around, you are likely to be deprecating the versions in boost::, and so the change in geometric_distribution would fit into the same category. -- Jeremiah Willcock
AMDG On 1/23/2011 4:52 PM, Jeremiah Willcock wrote:
I noticed that discrepancy between Boost.Random and usual definitions of geometric_distribution as well, but forgot to report it; I would prefer to have the standard definition rather than the current one. I support your second option, as long as people don't have both "using namespace boost;" and "using namespace boost::random;" in the same code.
This is unlikely with the current library, since everything useful to users is in namespace boost. Even if it does happen it will produce a compilation error, so it isn't as bad as it could be.
Since you state that you will be moving things around, you are likely to be deprecating the versions in boost::, and so the change in geometric_distribution would fit into the same category.
Okay. Everyone who's spoken up seems to favor this, so I've gone with it. I had to do the same for lognormal_distribution as well. In Christ, Steven Watanabe
On Mon, 24 Jan 2011, Steven Watanabe wrote:
Since you state that you will be moving things around, you are likely to be deprecating the versions in boost::, and so the change in geometric_distribution would fit into the same category.
Okay. Everyone who's spoken up seems to favor this, so I've gone with it. I had to do the same for lognormal_distribution as well.
Could you please post to the list when you get the renaming and this fix committed? I would like to update my code (in Boost) to match. -- Jeremiah Willcock
AMDG On 1/24/2011 1:30 PM, Jeremiah Willcock wrote:
On Mon, 24 Jan 2011, Steven Watanabe wrote:
Since you state that you will be moving things around, you are likely to be deprecating the versions in boost::, and so the change in geometric_distribution would fit into the same category.
Okay. Everyone who's spoken up seems to favor this, so I've gone with it. I had to do the same for lognormal_distribution as well.
Could you please post to the list when you get the renaming and this fix committed? I would like to update my code (in Boost) to match.
It's done in the trunk, but you might want to hold off on updating your code. I don't want to merge this to the release branch until I've gotten everything updated and I still have half a dozen distributions to update. In Christ, Steven Watanabe
On Mon, 24 Jan 2011, Steven Watanabe wrote:
AMDG
On 1/24/2011 1:30 PM, Jeremiah Willcock wrote:
On Mon, 24 Jan 2011, Steven Watanabe wrote:
Since you state that you will be moving things around, you are likely to be deprecating the versions in boost::, and so the change in geometric_distribution would fit into the same category.
Okay. Everyone who's spoken up seems to favor this, so I've gone with it. I had to do the same for lognormal_distribution as well.
Could you please post to the list when you get the renaming and this fix committed? I would like to update my code (in Boost) to match.
It's done in the trunk, but you might want to hold off on updating your code. I don't want to merge this to the release branch until I've gotten everything updated and I still have half a dozen distributions to update.
Should I wait to update code that is itself in the trunk? -- Jeremiah Willcock
AMDG On 1/24/2011 9:00 PM, Jeremiah Willcock wrote:
On Mon, 24 Jan 2011, Steven Watanabe wrote:
On 1/24/2011 1:30 PM, Jeremiah Willcock wrote:
Could you please post to the list when you get the renaming and this fix committed? I would like to update my code (in Boost) to match.
It's done in the trunk, but you might want to hold off on updating your code. I don't want to merge this to the release branch until I've gotten everything updated and I still have half a dozen distributions to update.
Should I wait to update code that is itself in the trunk?
You can go ahead. I'm close enough now, that I'm fairly confident that I'll have it ready for 1.47. I've finished all the distributions, and I just have 5 adapters + random_number_generator + independent_bits_engine + generate_canonical left. In Christ, Steven Watanabe
On 01/23/2011 04:40 PM, Steven Watanabe wrote:
AMDG
Over the last year I've been (slowly) updating Boost.Random to match the new standard. Unfortunately I just hit a problem that I don't have a good solution for. In the standard, the geometric_distribution is defined as $p(i) = p(1-p)^{i}$, with i >= 0. In the current Boost.Random code it is $p(i) = (1-p) p^{i-1}$, with i >= 1. The definition in the standard makes more sense, but I don't want to introduce a breaking change. I can see two options here:
* Leave the implementation alone and put a big warning in the documentation saying that the behavior doesn't match the standard.
That's not good. Why would anyone want to use it after the documentation said that?
* Since I'm also moving things into namespace boost::random, I can use the new definition and leave a version with the old behavior in namespace boost instead of having a using declaration. I'd still have to add a warning that boost::geometric_distribution isn't the same as boost::random::geometric_distribution.
That's better than the other, but I'd like to hear about the downside of a breaking change. Patrick
AMDG On 1/23/2011 10:26 PM, Patrick Horgan wrote:
On 01/23/2011 04:40 PM, Steven Watanabe wrote:
Over the last year I've been (slowly) updating Boost.Random to match the new standard. Unfortunately I just hit a problem that I don't have a good solution for. In the standard, the geometric_distribution is defined as $p(i) = p(1-p)^{i}$, with i >= 0. In the current Boost.Random code it is $p(i) = (1-p) p^{i-1}$, with i >= 1. The definition in the standard makes more sense, but I don't want to introduce a breaking change. I can see two options here:
* Leave the implementation alone and put a big warning in the documentation saying that the behavior doesn't match the standard.
That's not good. Why would anyone want to use it after the documentation said that?
It isn't unusable. It just implements a different variation of the distribution.
* Since I'm also moving things into namespace boost::random, I can use the new definition and leave a version with the old behavior in namespace boost instead of having a using declaration. I'd still have to add a warning that boost::geometric_distribution isn't the same as boost::random::geometric_distribution.
That's better than the other, but I'd like to hear about the downside of a breaking change.
The behavior changes, but the interface doesn't. If anyone is using this distribution and updates Boost, their code will continue to compile and run, but will produce the wrong results. In Christ, Steven Watanabe
On Mon, Jan 24, 2011 at 1:40 AM, Steven Watanabe <watanabesj@gmail.com> wrote:
AMDG
Over the last year I've been (slowly) updating Boost.Random to match the new standard. Unfortunately I just hit a problem that I don't have a good solution for. In the standard, the geometric_distribution is defined as $p(i) = p(1-p)^{i}$, with i >= 0. In the current Boost.Random code it is $p(i) = (1-p) p^{i-1}$, with i >= 1. The definition in the standard makes more sense, but I don't want to introduce a breaking change. I can see two options here:
* Leave the implementation alone and put a big warning in the documentation saying that the behavior doesn't match the standard. * Since I'm also moving things into namespace boost::random, I can use the new definition and leave a version with the old behavior in namespace boost instead of having a using declaration. I'd still have to add a warning that boost::geometric_distribution isn't the same as boost::random::geometric_distribution.
I vote for this one. It's like to have a new major version of Random.... And as most of the new major versions of a library it has not necessarily to be backward compatible (e.g., Boost.Signals vs Boost.Signals2 just to remain in the Boost family). Best, -- Marco
Steven Watanabe-4 wrote:
AMDG
A * Leave the implementation alone and put a big warning in the documentation saying that the behavior doesn't match the standard. B * Since I'm also moving things into namespace boost::random, I can use the new definition and leave a version with the old behavior in namespace boost instead of having a using declaration. I'd still have to add a warning that boost::geometric_distribution isn't the same as boost::random::geometric_distribution.
Neither one is particularly appealing. Any thoughts? Better ideas?
Don't breaking code is the best we can do. Option B allows to don't break user code, and if the non standard behavior is marked as deprecated (and is a compile warning can be generated) the user can move smoothly to the standard behavior. Best, Vicente -- View this message in context: http://boost.2283326.n4.nabble.com/random-geometric-distribution-backwards-c... Sent from the Boost - Dev mailing list archive at Nabble.com.
participants (5)
-
Jeremiah Willcock -
Patrick Horgan -
sguazt -
Steven Watanabe -
Vicente Botet