REMOVE reconstruction and 2d skeleton.

[petrasvestartas]

Temporary changes:

• removal of reconstruction
• removal 2d skeleton
• added cmake flag to increase memory.

[jf---]

Whoa @petrasvestartas @tomvanmele; both straight skeleton and reconstruction are very useful tools. IIRC @romanarust worked quite a bit on straight skeleton and I think it's important too.

The changelog doesn't mention a motivation why to remove these methods -- could you enlighten us?

[tomvanmele]

we're changing the build system. everything is working well but there are some memory problems with the conda builds for windows. these are just temporary test releases to work through the problem. everything will be resolved by end of day tomorrow.

all of this is necessary to be able to switch to latest cgal, resolve slow resolution times with conda installs, and general problems with some of the dependencies on some systems...

[tomvanmele]

and as an added bonus, after that compas_cgal will be available directly from PyPI.

[jf---]

Thanks for the heads-up @tomvanmele

just temporary test releases to work through the problem

I see; but somewhat confusing for such operations to land in the main branch over a refactor branch?

we're changing the build system

CGAL is a bit of a monster so that figures

switch to latest cgal

Wow!

Thanks for providing context!

[petrasvestartas]

@jf---

I see; but somewhat confusing for such operations to land in the main branch over a refactor branch?

In the current setup, there is no other way to test conda releases other than from the main branch when releases are made. At least this is how current setup is made. Unless we will make a specific branch for conda releases.

Also, we can do a dummy repo, but we do not want to populate conda with temporary packages, as the name cannot be removed same as PYPI.

[tomvanmele]

@petrasvestartas after we get the current build stuff sorted out, perhaps we can start working on a system of separate branches for releases etc.

[jf---]

Hi @petrasvestartas

(great to see the compas_cgal its a part of compas I'm extra fond of ;) )

test conda releases

Ah, I suppose its the emphasis on the releases (vs local builds)
Are you aware of https://github.com/nektos/act which allow for GH action to be ran locally ?

dummy repo / temp packages

Oh gosh no, but that does clarify things.

Did the yanked modules stall the refactor?

[tomvanmele]

Did the yanked modules stall the refactor?

those modules created memory problems during the build process for windows, but only on conda. locally there were no issues. @petrasvestartas is working on a solution with the codebase split up into smaller chunks, and we will try to control the number of parallel processes during the windows builds if we can...

[petrasvestartas]

@jf--- Everything works, that worked before, including the reconstruction and 2d skeleton.

Hooray!

[jf---]

Amazing... is pybind11 -> nanobind a worthy upgrade?

[petrasvestartas]

ReadMe file explains the history and reasoning:

https://github.com/wjakob/nanobind

[lrineau]

@petrasvestartas What was the issue with CGAL 2D skeleton and reconstruction?

This PR was supposed to be temporary, but according to the changelog, releases including le last one (v0.8.0) have been shipped with those two features removed.

Maybe the CGAL project was help you reintroduce them. Please let us know (starting here).

[petrasvestartas]

@lrineau very nice to hear from cgal developer:)

This was a temporary removal. It is currently working and brought back to the main repository.
C++ project was hitting the limit of maximum RAM used in heap memory during CMake build. This was a temporary removal to check if conda would build the package and it did with almost the maximum cloud memory limit available.

Then decision was made to split the project into multiple targets since boost, eigen, cgal and nanobind are heavily templated thus needs more memory. Also conda is now build without multithreading to reduce the memory needed.

Local builds had no problem, only conda was hitting the memory limit.

It is for sure a bit strange, because we are using just a very few cgal methods.

[tomvanmele]

@petrasvestartas the changelog is now a bit confusing indeed. we wrote that the methods were removed, albeit temporarily, but never wrote that they were reintroduced. perhaps we should update it retroactively...