Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameterization: cleanup #7816

Merged
merged 14 commits into from
Nov 29, 2023
Merged

Conversation

afabri
Copy link
Member

@afabri afabri commented Oct 20, 2023

Summary of Changes

LSCM parameterization was prepared to fall back to code forked from OpenNL some years back. As it is 10 times slower, we better depend on Eigen just as the other parameterization methods do.

Todo

  • Currently one file remains, but that fills Eigen matrices and calls an Eigen solver. No need to be in a namespace OpenNL.
  • cleanup usage.hhml
  • changes.md

Release Management

  • Affected package(s): Surface_mesh_parameterization
  • Issue(s) solved (if any): fix Non existing link to open numerical library #7772
  • Link to compiled documentation (obligatory for small feature) wrong link name to be changed
  • License and copyright ownership: unchanged

Copy link
Contributor

@albert-github albert-github left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that the file linear_solver.h contains still some non OpenNL related information.
Why isn't this file moved away from here and isn't the complete OpenNL directory removed?

@afabri
Copy link
Member Author

afabri commented Oct 20, 2023

I can see that the file linear_solver.h contains still some non OpenNL related information. Why isn't this file moved away from here and isn't the complete OpenNL directory removed?

That is the first Todo. Be patient.

@sloriot sloriot added Batch_1 First Batch of PRs under testing Under Testing and removed Batch_1 First Batch of PRs under testing labels Oct 24, 2023
@sloriot
Copy link
Member

sloriot commented Nov 2, 2023

Successfully tested in CGAL-6.0-Ic-96

@lrineau
Copy link
Member

lrineau commented Nov 2, 2023

@afabri This PR is tested, but there is still a TODO entry.

Currently one file remains, but that fills Eigen matrices and calls an Eigen solver. No need to be in a namespace OpenNL.

Is that something you plan to do?

@lrineau lrineau added the TODO label Nov 2, 2023
@MaelRL MaelRL removed the Tested label Nov 2, 2023
@afabri
Copy link
Member Author

afabri commented Nov 2, 2023

@afabri This PR is tested, but there is still a TODO entry.

Currently one file remains, but that fills Eigen matrices and calls an Eigen solver. No need to be in a namespace OpenNL.

Is that something you plan to do?

yes

@sloriot sloriot added the Batch_2 Second Batch of PRs under testing label Nov 9, 2023
@afabri afabri removed the TODO label Nov 13, 2023
Copy link
Contributor

@albert-github albert-github left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the risk of being impatient...

I still see the word OpenNL (in different iform of lower / upper case) still in a number of files a.o.:

Installation/LICENSE
Maintenance/deb/sid/debian/copyright
Maintenance/deb/squeeze/debian/copyright
Maintenance/deb/wheezy/debian/copyright
Maintenance/infrastructure/cgal.geometryfactory.com/reference-platforms/i686_Linux-2.6_g++-4.1.2_CentOS-5.1/CMakeCache.txt
Maintenance/infrastructure/cgal.geometryfactory.com/reference-platforms/i686_Linux-2.6_g++-4.1.2_CentOS-5.1-O2/CMakeCache.txt
Maintenance/infrastructure/cgal.geometryfactory.com/reference-platforms/i686_Linux-2.6_g++-4.1.2_CentOS-5.1-O3/CMakeCache.txt
Maintenance/infrastructure/cgal.geometryfactory.com/reference-platforms/x86_64_Linux-x.y_IntelCompiler-15.0_CentOS-7.x/CMakeCache.txt
Maintenance/infrastructure/magritte.geometryfactory.com/reference_platforms/x86-64_Darwin-13.0_Apple-clang-5.0_Release/CMakeCache.txt
Maintenance/infrastructure/magritte.geometryfactory.com/reference_platforms/x86-64_Darwin-13.0_Apple-clang-5.0_Release-cpp11/CMakeCache.txt
Maintenance/infrastructure/magritte.geometryfactory.com/reference_platforms/x86-64_Darwin-13.0_Apple-clang-5.1_Release-LEDA-without-GMP/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_g++-4.5-branch_CXX0X/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_g++-4.5-branch_Release/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_g++-4.8/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_g++-4.8_ansi/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_g++-4.8_CXX0X/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_g++-4.8_CXXDEBUG/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_g++-4.8_m32/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_g++-4.8_MATCHING-BUG-6/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_g++-4.8_Release/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_g++-4.8_Release-LEDA/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_g++-trunk_CXX0X/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_IntelCompiler-14.0-with-g++-4.8-STL/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_IntelCompiler-14.0-with-g++-4.8-STL_strict-ansi/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora19_llvm-clang-with-g++-4.8/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora_IntelCompiler-17/CMakeCache.txt
Maintenance/infrastructure/renoir.geometryfactory.com/reference-platforms/x86-64_Linux-Fedora_IntelCompiler-17-with-assertions/CMakeCache.txt
Scripts/developer_scripts/check_licenses
Scripts/scripts/cgal_create_CMakeLists
  in comment
Solver_interface/doc/Solver_interface/Solver_interface.txt
Surface_mesh_parameterization/doc/Surface_mesh_parameterization/Surface_mesh_parameterization.txt
  a bit dubious as it is now removed again, maybe an extra remark that it has been removed as of ...

@afabri
Copy link
Member Author

afabri commented Nov 14, 2023

I left the files in the directory Maintenance unchanged. @lrineau is that ok?

@afabri
Copy link
Member Author

afabri commented Nov 14, 2023

Surface_mesh_parameterization/doc/Surface_mesh_parameterization/Surface_mesh_parameterization.txt
a bit dubious as it is now removed again, maybe an extra remark that it has been removed as of ...

I added a sentence that it was later removed.

@sloriot sloriot added Under Testing and removed Batch_2 Second Batch of PRs under testing labels Nov 21, 2023
@sloriot
Copy link
Member

sloriot commented Nov 29, 2023

Successfully tested in CGAL-6.0-Ic-116

Copy link

This pull-request was previously marked with the label Tested, but has been modified with new commits. That label has been removed.

@github-actions github-actions bot removed the Tested label Nov 29, 2023
@lrineau lrineau added rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' Tested labels Nov 29, 2023
I checked the compilation of the documentation locally.
@github-actions github-actions bot removed the Tested label Nov 29, 2023
Copy link

This pull-request was previously marked with the label Tested, but has been modified with new commits. That label has been removed.

@lrineau
Copy link
Member

lrineau commented Nov 29, 2023

I left the files in the directory Maintenance unchanged. @lrineau is that ok?

I removed them in a later commit (c19cd15).

@lrineau
Copy link
Member

lrineau commented Nov 29, 2023

Here are the last occurrences of opennl in CGAL:

[lrineau@fernand]~/Git/cgal-master% ack opennl -i  ^build*                                
Installation/CHANGES.md
2477:-   `LSCM_parameterizer_3` now uses by default Eigen instead of OpenNL
4725:    Added Jacobi and SSOR preconditioners to OpenNL solver, which makes

Solver_interface/doc/Solver_interface/Solver_interface.txt
128:on \taucs, \lapack, \blas and OpenNL. Gaël Guennebaud

Surface_mesh_parameterization/doc/Surface_mesh_parameterization/Surface_mesh_parameterization.txt
537:Pierre Alliez and Bruno Lévy. Bruno Lévy added OpenNL support to the package.
550:In CGAL 6.0 this package no longer uses the fork of OpenNL that was distributed with CGAL

so:

  • the changelog,
  • and two "history" sections in the manual.

Thas is okay.

@lrineau lrineau self-assigned this Nov 29, 2023
@lrineau lrineau merged commit 23e3648 into CGAL:master Nov 29, 2023
9 checks passed
@lrineau lrineau removed the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Nov 30, 2023
@lrineau lrineau deleted the Parameterization-no_opennl-GF branch November 30, 2023 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non existing link to open numerical library
5 participants