KratosMultiphysics / Kratos

Kratos Multiphysics (A.K.A Kratos) is a framework for building parallel multi-disciplinary simulation software. Modularity, extensibility and HPC are the main objectives. Kratos has BSD license and is written in C++ with extensive Python interface.
https://kratosmultiphysics.github.io/Kratos/
Other
1.03k stars 245 forks source link

License of external library: "triangle" #9158

Closed armingeiser closed 3 years ago

armingeiser commented 3 years ago

Description A project partner is evaluating Kratos for commercial use and while checking the licenses of the external libraries in the core, they stumbled upon the license of the "triangle" (external_libraries/triangle). The license seems to be self-written by the author and does allow commercial use, but only if the author is contacted first. This might complicate the process of allowing the use of Kratos commercially by the legal department of larger companies.

The library seems not to be used in the core, but by MeshingApplicationand DelaunayMeshingApplication.

Question:

Scope

philbucher commented 3 years ago

It is used in the Core :)

@KratosMultiphysics/technical-committee

armingeiser commented 3 years ago

I just see now as well, its used in kratos/utilities/delaunator_utilities.cpp as well...

philbucher commented 3 years ago

Yes the dependency was added in #5169

It is not a big dependency and in the worst case could be refactored (I remember the change was controversial anyway) but it would be some work,,,

roigcarlo commented 3 years ago

If is only that file, it can be excluded from a regular compilation. In this case is particularly easy as its only used in vicente's own code. @philbucher I think we can just remove both cpp from the compilation and that's it (maybe a ugly #ifdef in the modelers register...)

In my opinion the way to go would be to either create a different compilation object where all this utilities and "extra" components that make use of external libraries exists (which can turned on/off with a compilation option) or either create a different Kratos application for them (call it KratosExtendedCore).

I would go for the first one, tbh.

loumalouomega commented 3 years ago

If is only that file, it can be excluded from a regular compilation. In this case is particularly easy as its only used in vicente's own code. @philbucher I think we can just remove both cpp from the compilation and that's it (maybe a ugly #ifdef in the modelers register...)

In my opinion the way to go would be to either create a different compilation object where all this utilities and "extra" components that make use of external libraries exists (which can turned on/off with a compilation option) or either create a different Kratos application for them (call it KratosExtendedCore).

I would go for the first one, tbh.

I just want to say that originally I was using Delaunator, a MIT library, but @KratosMultiphysics/technical-committee recommended to use triangle (you can see the discussion in the PR), because was already in the repo. I have no problem in going back to Delaunator, which is faster for small meshes (and I did this for the mortar segmentation)

loumalouomega commented 3 years ago

If is only that file, it can be excluded from a regular compilation. In this case is particularly easy as its only used in vicente's own code. @philbucher I think we can just remove both cpp from the compilation and that's it (maybe a ugly #ifdef in the modelers register...) In my opinion the way to go would be to either create a different compilation object where all this utilities and "extra" components that make use of external libraries exists (which can turned on/off with a compilation option) or either create a different Kratos application for them (call it KratosExtendedCore). I would go for the first one, tbh.

I just want to say that originally I was using Delaunator, a MIT library, but @KratosMultiphysics/technical-committee recommended to use triangle (you can see the discussion in the PR), because was already in the repo. I have no problem in going back to Delaunator, which is faster for small meshes (and I did this for the mortar segmentation)

Do you want me to revert to Delaunator?

philbucher commented 3 years ago

I would wait

maceligueta commented 3 years ago

This was discussed long ago and, as far as I remember, we decided there was no problem in having 'triangle' in the repository. As opposed to that, 'Tetgen' is a different story and could not be included (3D Delaunay).

loumalouomega commented 3 years ago

This was discussed long ago and, as far as I remember, we decided there was no problem in having 'triangle' in the repository. As opposed to that, 'Tetgen' is a different story and could not be included (3D Delaunay).

The "LICENSE" in the current code (doesn't matter the current license, but the one we "copied"):

These programs may be freely redistributed under the condition that the copyright notices (including the copy of this notice in the code comments and the copyright notice printed when the `-h' switch is selected) are not removed, and no compensation is received. Private, research, and institutional use is free. You may distribute modified versions of this code UNDER THE CONDITION THAT THIS CODE AND ANY MODIFICATIONS MADE TO IT IN THE SAME FILE REMAIN UNDER COPYRIGHT OF THE ORIGINAL AUTHOR, BOTH SOURCE AND OBJECT CODE ARE MADE FREELY AVAILABLE WITHOUT CHARGE, AND CLEAR NOTICE IS GIVEN OF THE MODIFICATIONS. Distribution of this code as part of a commercial system is permissible ONLY BY DIRECT ARRANGEMENT WITH THE AUTHOR. (If you are not directly supplying this code to a customer, and you are instead telling them how they can obtain it for free, then you are not required to make any arrangement with me.)

If you use Triangle, and especially if you use it to accomplish real work, I would like very much to hear from you. A short letter or email (to jrs@cs.berkeley.edu) describing how you use Triangle will mean a lot to me. The more people I know are using this program, the more easily I can justify spending time on improvements and on the three-dimensional successor to Triangle, which in turn will benefit you. Also, I can put you on a list to receive email whenever a new version of Triangle is available.

If you use a mesh generated by Triangle or plotted by Show Me in a publication, please include an acknowledgment as well. And please spell Triangle with a capital T'! If you want to include a citation, use Jonathan Richard Shewchuk, ``Triangle: Engineering a 2D Quality Mesh Generator and Delaunay Triangulator,'' in Applied Computational Geometry: Towards Geometric Engineering (Ming C. Lin and Dinesh Manocha, editors), volume 1148 of Lecture Notes in Computer Science, pages 203-222, Springer-Verlag, Berlin, May 1996. (From the First ACM Workshop on Applied Computational Geometry.)'

Jonathan Richard Shewchuk July 27, 2005

philbucher commented 3 years ago

Distribution of this code as part of a commercial system is permissible ONLY BY DIRECT ARRANGEMENT WITH THE AUTHOR.

I am no lawyer but since we are distributing triangle it sounds to me like we would need to contact him @maceligueta do you remember what lead to the consensus?

maceligueta commented 3 years ago

I think @RiccardoRossi dedicated time to this and made a summary to the Technical Committee.

RiccardoRossi commented 3 years ago

well, the triangle was included because to our understanding there was no issue with the license. @roigcarlo we could send a mail to shewchuck (who will probably not answer) from our "institutinal" kratos account informing him.

Tetgen on the other hand should never be distributed, under no circunstances. If by some kind of error it is present in the repository it should be removed ASAP.

having said this i think that the proposal of @roigcarlo of conditionally removing it is more than fair to me. @armingeiser would that solve the issue?

@philbucher am i answering convincingly?

loumalouomega commented 3 years ago

having said this i think that the proposal of @roigcarlo of conditionally removing it is more than fair to me. @armingeiser would that solve the issue?

(and therefore I need to replace the library with another one, as I did the first time?)

RiccardoRossi commented 3 years ago

no, just thr feature gets inhabilitated...

On Fri, Oct 15, 2021, 8:35 AM Vicente Mataix Ferrándiz < @.***> wrote:

having said this i think that the proposal of @roigcarlo https://github.com/roigcarlo of conditionally removing it is more than fair to me. @armingeiser https://github.com/armingeiser would that solve the issue?

(and therefore I need to replace the library with another one, as I did the first time?)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/KratosMultiphysics/Kratos/issues/9158#issuecomment-944038980, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5PWEJZ6UHXVOEAGOHZFDTUG7DUTANCNFSM5EVTXBTQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

loumalouomega commented 3 years ago

no, just thr feature gets inhabilitated...

That feature is used in the IGAApp @tteschemacher and in the Contact, why not just replace with a library licence compatible that can get the job done?

roigcarlo commented 3 years ago

(and therefore I need to replace the library with another one, as I did the first time?)

No you won't have. As @RiccardoRossi says I am speaking of making some kind of USE_TRIANGLE_TPL or whatever that completely wipes out triangle and all code that depends on it. In fact I since we already have a couple of this I would propose to create a -USE_PURE_BSD4 or kind of flag that automatically disables the offending parts of the code.

@roigcarlo we could send a mail to shewchuck

Ok, I will try to reach him but I think in this case its of no use. The problem here is that part of its license enters in conflict with what we allow with BSD (but not on our end)

I am no lawyer but since we are distributing triangle it sounds to me like we would need to contact him @maceligueta do you remember what lead to the consensus?

I disagree with that. For what I understand we are not the ones making a commercial code out of triangle, @armingeiser partners are, and such they are the ones that need to reach an agreement. If it were with us that would imply that anyone using Kratos could potentially use triangle for commercial code, which among other things would create a funny situation to say the least.

That feature is used in the IGAApp @tteschemacher and in the Contact, why not just replace with a library licence compatible that can get the job done?

Also fine to me, if it can be done.

philbucher commented 3 years ago

I disagree with that. For what I understand we are not the ones making a commercial code out of triangle, @armingeiser partners are, and such they are the ones that need to reach an agreement. If it were with us that would imply that anyone using Kratos could potentially use triangle for commercial code, which among other things would create a funny situation to say the least.

my opinion

I think this is potentially a or even the blocker to have Kratos used in an industrial environment. From my POV we should stick to pure BSD4 at least for the Core, and have all potentially conflicting things in Apps. Important IMO now is the time to decide this, as it is still not too much effort to make the Core pure BSD4 again (by doing what @loumalouomega suggests basically). It will get more and more difficult down the line with more and more things added

armingeiser commented 3 years ago

For what I understand we are not the ones making a commercial code out of triangle

Thats my point of view as well.

From my POV we should stick to pure BSD4 at least for the Core,

I completely agree here. The Kratos community keeps pointing out BSD4 license of Kratos and the possibility of commercial use without restrictions. With such a external library with contradicting license this is not true anymore.

If we go for a flag that switches such differently licenced features off, i would fo gor something like -USE_NON_BSD4_FEATURES and set it to FALSE by default. Like this explicit action has to be taken by someone who wants to use non-BSD4 code.

In this case if there are BSD4 compatible alternatives and even people willing to replace the problematic code, this would be even better.

RiccardoRossi commented 3 years ago

on my side i reread the license of triangle, and i now agree with the idea of removing triangle and substituting it by another library as Vicente suggests. @loumalouomega sorry...you were right in your position since the beginning.

We could do the deactivation as a flag as a short term solution, and the substitution of the library longer term.

what features will we lose by doing this?

loumalouomega commented 3 years ago

on my side i reread the license of triangle, and i now agree with the idea of removing triangle and substituting it by another library as Vicente suggests. @loumalouomega sorry...you were right in your position since the beginning.

We could do the deactivation as a flag as a short term solution, and the substitution of the library longer term.

what features will we lose by doing this?

In my tests, the fastest was this one: https://github.com/delfrrr/delaunator-cpp, faster for small meshes than triangle, and slightly slower for large meshes

RiccardoRossi commented 3 years ago

@loumalouomega but will this only connect the cloud of nodes, or will this also do constrained delaunay?

loumalouomega commented 3 years ago

@loumalouomega but will this only connect the cloud of nodes, or will this also do constrained delaunay?

For my mortar segmentation, that's enough. But you are right, if we want to replace triangle we need to look for something completely equivalent. I don't know if @tteschemacher just needs to generate a mesh from a cloud of nodes or actually requires the constrained Delaunay

loumalouomega commented 3 years ago

At the moment, I checked different libraries, but I chose delaunator because it was easier to use, I will list the CDT libraries I tried:

RiccardoRossi commented 3 years ago

the position of the @KratosMultiphysics/technical-committee is that triangle needs to be removed from the core, and that this needs to happen (either by deactivation, or by replacement on the lib).

this also needs to happen fast, before the next release which should be due by the end of november

@loumalouomega but will this only connect the cloud of nodes, or will this also do constrained delaunay?

For my mortar segmentation, that's enough. But you are right, if we want to replace triangle we need to look for something completely equivalent. I don't know if @tteschemacher just needs to generate a mesh from a cloud of nodes or actually requires the constrained Delaunay

Well, the current interface does not allow one to use the CDT capabilities correct? if that is so, then we could get rid of triangle without affecting any capability since the cdt cannot be used as of now. is this a correct interpretation?

loumalouomega commented 3 years ago

the position of the @KratosMultiphysics/technical-committee is that triangle needs to be removed from the core, and that this needs to happen (either by deactivation, or by replacement on the lib).

this also needs to happen fast, before the next release which should be due by the end of november

@loumalouomega but will this only connect the cloud of nodes, or will this also do constrained delaunay?

For my mortar segmentation, that's enough. But you are right, if we want to replace triangle we need to look for something completely equivalent. I don't know if @tteschemacher just needs to generate a mesh from a cloud of nodes or actually requires the constrained Delaunay

Well, the current interface does not allow one to use the CDT capabilities correct? if that is so, then we could get rid of triangle without affecting any capability since the cdt cannot be used as of now. is this a correct interpretation?

In my code CDT is not required, and in the one of @tteschemacher I think neither (as it uses my code as well), so we can for the moment replace it and with calm analyse the best CDT alternative.

maceligueta commented 3 years ago

@loumalouomega what do you think about QHull? http://www.qhull.org/html/index.htm I am asking because I even found distributed memory meshers which use QHull inside.

loumalouomega commented 3 years ago

@loumalouomega what do you think about QHull? http://www.qhull.org/html/index.htm I am asking because I even found distributed memory meshers which use QHull inside.

"Qhull does not support constrained Delaunay triangulations, triangulation of non-convex surfaces, mesh generation of non-convex objects, or medium-sized inputs in 9-D and higher. "

But I imagine you are interested in for your "FD DEM method"

maceligueta commented 3 years ago

I was actually thinking of PFEM Fluid

loumalouomega commented 3 years ago

I was actually thinking of PFEM Fluid

Then you need a CDT?

maceligueta commented 3 years ago

No, the PFEM Fluid never used a Constrained Delaunay Tesselation. Just the regular one. If you are just considering CDT then forget my question, I understand that you never needed to test QHull.

loumalouomega commented 3 years ago

No, the PFEM Fluid never used a Constrained Delaunay Tesselation. Just the regular one. If you are just considering CDT then forget my question, I understand that you never needed to test QHull.

I don't need it, and neither @tteschemacher (I think), the constraint comes from @RiccardoRossi (I imagine he wants to replace with something equivalent)

tteschemacher commented 3 years ago

Hey, thanks for checking out on this. I myself haven't used triangle much. I have always used the Delaunator, as you correctly said. This one was very simple and worked really well.

I remember triangle was used by @rubenzorrilla and @dagmawibekel for the tessellation of the surfaces in preparation of the iga surfaces for embedded CFD. Not sure if this is still the case. If so, this is part of the core.

armingeiser commented 3 years ago

@roigcarlo @loumalouomega thanks for solving this!