Closed tirthasheshpatel closed 2 years ago
Additional comment on the license topic: We were given the permission to publish UNU.RAN 1.8.0 under the BSD license. We then reported a bug and the UNURAN maintainers told us that we can use the new release to fix it. So I do not see a problem if GPL is mentioned as long it is clear that the code is written by the maintainers or taken from someone else that allows to distribute it freely (like in spectfunc/cgamma.c).
Only spectfunc/hypot.c might be a problem since it uses code that someone else published under GPL
Even for hypot.c, it looks like Joseph added the comment about the GPL licence and it is not clear if the authors mentioned imposed any licence restrictions. It is a very simple code to compute sqrt(x**2 + y**2)
in a way that is numerically stable.
Copied and renamed from gsl_hypot into _unur_hypot
* by Josef Leydold, Tue Nov 1 11:17:37 CET 2011
*
* Copyright (C) 1996, 1997, 1998, 1999, 2000, 2007 Gerard Jungman, Brian Gough, Patrick Alken
though I don't know how the issue with constant densities is fixed
In line 396 of ./unuran/src/methods/pinv_newton.h
(function _unur_pinv_newton_create
), the derivative of the PDF was called if the density didn't change. But dPDF hasn't been listed as a requirement for pinv. Which is why the code segfaulted. This has been fixed by filling in infinity. Here's the diff:
There are references to GPL that we need to discuss.
Thanks for noticing that! I will look into the GPLed code.
Additional comment on the license topic: We were given the permission to publish UNU.RAN 1.8.0 under the BSD license. We then reported a bug and the UNURAN maintainers told us that we can use the new release to fix it. So I do not see a problem if GPL is mentioned as long it is clear that the code is written by the maintainers or taken from someone else that allows to distribute it freely (like in spectfunc/cgamma.c).
Only spectfunc/hypot.c might be a problem since it uses code that someone else published under GPL
I would then update both Readme and license.txt to have a clear history of this. I would still remove the mention of GPL in the files because it could confuse automatic tools looking into licenses. It would be bad if it flagged this repo as GPL.
There are references to GPL that we need to discuss.
I looked into this. We don't use any of the GPLed code other than the one written by the authors (we should be safe to use that and remove those references to GPL)
We don't use any of the GPLed code other than the one written by the authors (we should be safe to use that and remove those references to GPL)
I agree.
Do you have an idea on how to proceed with hypot
? I did not understand if the NumPy issue https://github.com/numpy/numpy/issues/9567 is relevant in our situation
Do you have an idea on how to proceed with
hypot
?
I think https://github.com/numpy/numpy/issues/9567 is still relevant. To avoid using the GPL code, I just copied NumPy's hypot and switch to it for 32-bit windows (which causes problems for inf input). As discussed, we have permission to use other code. I will update the SciPy PR to check if the latest changes work.
Good news: It looks like the failures in the SciPy PR are unrelated to your changes (timeouts or the known failure of truncnorm)
(It looks funny that we replace the hypot code with the same code from np (modulo variable names :))
It looks funny that we replace the hypot code with the same code from np (modulo variable names
Yeah, I have heard even simple GPLed code can gen one into a lot of trouble. Now, no one can blame us for using GPL code :)
@chrisb83 This has been sitting around for a while now. Can you merge? I will update scipy/scipy#15798 and we can merge that one too.
Sure, I did not know that I am allowed to merge in this project. Since there are no open points, I will do it right away.
Thanks @chrisb83!
UNU.RAN 1.9.0 was released yesterday. This release contains a few bug fixes. In particular:
NumericalInversePolynomial
caused by uniform densities.INFINITY
withUNUR_INFINITY
(but we have updated to C99 so this should not change anything)Note to reviewers: I will propose a pull request on SciPy main cloning this branch to check if it builds on all the systems and resolves the segfault. Once that's working we can merge this and then merge the PR on SciPy main.
The SciPy PR is up: scipy/scipy#15798