Incompatible license with STEPS future goals

[iahepburn]

Dear @severinstrobl ,

Firstly, thank you for this awesome overlap library which we use in the STEPS project (https://github.com/CNS-OIST/STEPS), specifically for calculating overlap between simulated vesicles and our tetrahedral meshes, as described in: https://www.nature.com/articles/s42003-024-06276-5

When including your project under the GPL this means that STEPS also has to be released under GPL, as it is currently. However, this is incompatible with future plans of STEPS, which include being released as open source software for neural simulation by the Blue Brain Project (https://www.epfl.ch/research/domains/bluebrain/) or its successor. Blue Brain project developer https://github.com/tristan0x will be able to provide more details on these goals.

So, this is quite an issue for us because we rely on your library yet with no license change we wouldn't be able to use it in the future and would have to come up with an alternative. By far our preference is to continue using your library, which would however require a change of license. So I just wanted to open this discussion and ask, would you be open in principle to potentially changing the license for the overlap library to one that is compatible with our future plans? We will provide more details on our plans, and can put forward licenses that would be compatible for us for your consideration if so. Thank you.

[severinstrobl]

Hi @iahepburn, happy to hear you find this library useful for your works. Funnily enough, there was a very similar request from a different group looking to combine this library with their own works. This resulted in a change of the development version of overlap to the MIT license (see #92). While it might still take some time for me to implement and integrate all the changes I would like to see for an official 0.2 release, the library itself is in a quite robust state. Is the MIT license compatible with your plans?

[tristan0x]

Thank you very much for your reply @severinstrobl
MIT license for the future 0.2 release would be perfect indeed.
We will take a look at the develop version of overlap. Do you plan to refactor the public API or will 0.2 look like the current state of develop?
Thank you again for this library

[severinstrobl]

@tristan0x: Please excuse the delayed response. I don't expect further changes to the public API for the 0.2 release, so basing your work on the current develop version should make the switch to the 0.2 release trivial.

[iahepburn]

Dear @severinstrobl ,

Thank you for the MIT license branch. We have already integrated this into STEPS and this is very helpful for us. However, we have noticed an occasional accuracy issue at small length scales. We typically operate at nm scales and the absolute volume overlap can be order 1e-25 or so. At these scales the overlap calculation sometimes fails (the previous library didn't have this issue). I can give one example in code:

    overlap::Vector  pos(1.7553357e-06, 4.2232066e-06, 5.8329073e-07);
    overlap::Sphere vesicle(pos, 20e-9);
    overlap::Vector vertex0(1.7503302395906002e-06, 4.2330364312997e-06, 5.961778422123901e-07);
    overlap::Vector vertex1(1.7438173901207002e-06, 4.222375361573301e-06, 5.9263766042144e-07);
    overlap::Vector vertex2(1.7394539738699001e-06, 4.2382759184772e-06, 6.009593818316999e-07);
    overlap::Vector vertex3(1.7544257028301e-06, 4.2350646020068004e-06, 5.840237397166e-07);
    overlap::Tetrahedron tet(vertex0, vertex1, vertex2, vertex3);
    overlap::Scalar ovlp = overlap::overlap_volume(vesicle, tet);
    

This calculation gives negative overlap.

We have tracked it down to the calculation of the normal vector using Newell's method:

overlap/include/overlap/overlap.hpp

Line 67 in 351d479

length > std::numeric_limits<Scalar>::epsilon()) {

, which seems to assume a length scale of 1. Since we are operating at length scales of ~1e-9 and the cross product is therefore order 1e-18 we believe a multiple of 1e-18 would be right for us here. The above code works with that modification (actually also with a much smaller modification). Firstly, do you agree with that scale? And, secondly, is there a way that we could add this scale to the library somehow? We can of course just modify the code that we use for now. Thank you.

[severinstrobl]

Hi @iahepburn, indeed you found a regression introduced in the development version of the library. The problem is not the initial scale of the sphere/tetrahedron, before processing a scaling of all coordinates resulting in a unit sphere centered at the origin is performed. Nevertheless, you are absolutely correct about the problem in the normal calculation you identified. I'm surprised this did not break any of the existing test cases.
Either way, thank you for providing the test case which helped me to quickly reproduce the issue!

[severinstrobl]

Hi @iahepburn, I merged the fix from #98 into the development branch, could you give this version a try?

[iahepburn]

Thank you @severinstrobl for this quick fix. Yes, all seems to be working well now. I think we can close the issue.