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

Update SuiteSparse to 7.5.1 with embedded Metis #125

Merged
merged 9 commits into from
Jan 22, 2024
Merged

Conversation

NeroBurner
Copy link
Collaborator

The following big changes have been done:

  • fixes Metis based segmentation fault #123
  • remove SuiteSparse::metis target as Metis is now a private dependency of CHOLMOD module
  • remove option METIS_DIR
  • always compile SuiteSparse_config with -DNTIMER disabling tic-toc support
  • compile CXSparse without complex support
  • remove SourceWrappers for CHOLMOD and CXSparse modules

The changes are split up into fitting commits with more information on the specific change.

Should we just remove the new not (by us) supported SuiteSparse modules to save space?

After the SuiteSparse 7.5.1 upgrade the following SourceWrappers error
because of missing referenced files. Remove them.

- amd: remove SourceWrappers/amd_global.c
- camd: remove SourceWrappers/camd_global.c
- UMFPACK: remove SourceWrappers/umfpack_gn_global.o.c
The upstream SuiteSparse 7.5.1 project now includes the metis project as
a subfolder of the `CHOLMOD` module with the name `SuiteSparse_metis`.
This copy is modified by the project to the point, that external metis
versions probably won't work anymore and the included copy only works
for the `CHOLMOD` module.

Therefore use Metis as private dependency and remove the
`SuiteSparse::metis` target from the installed targets.

The following changes have been done to the cmake project files:
- SuiteSparse: Metis is CHOLMOD private dependency now, don't install
- SuiteSparse: metis include only needed for CHOLMOD
- cholmod: glob c files, remove wrappers, use metis directly
- SPQR: remove metis linkage
- config: remove export of SuiteSparse::metis target, internal only
- remove SuiteSparse_LINKER_METIS_LIBS and SuiteSparse_EXPORTED_METIS_LIBS
Disable the SuiteSparse tic-toc timer. This functionality is primarily
used by debugging and examples. SuiteSparse still works without them.

As the timer support produces issues on `MSVC` disable them globally.
Since the upgrade to SuiteSparse 7.5.1 the `cs.h` file in upstream is
created by CMake from the `cs.h.in` template. To limit the CMake code
just use the checked in `cs.h`.

But the checked in `cs.h` has complex support enabled by default, which
is a problem for `MSVC` builds. So for now disable the complex support
in the checked in `cs.h` and also remove the possibility to change it
from CMake by options.

The following changes have been done to the CMake project:
- CXSparse: disable complex usage in cs.h
- cmake: CXSparse: use c files directly, not SourceWrappers
- cmake: CXSparse: no complex support, hard coded
- cmake: SuiteSparse: no complex support, remove option
@NeroBurner NeroBurner added the feature request Request for a new feature or improvement label Jan 22, 2024
@NeroBurner NeroBurner self-assigned this Jan 22, 2024
- fixes Metis based segmentation fault [#123](#123)
- remove `SuiteSparse::metis` target as Metis is now a private dependency of `CHOLMOD` module
- remove option `METIS_DIR`
- always compile `SuiteSparse_config` with `-DNTIMER` disabling tic-toc support
- compile `CXSparse` without complex support
- remove SourceWrappers for `CHOLMOD` and `CXSparse` modules
@NeroBurner
Copy link
Collaborator Author

squashed the log commits to have one clean commit that changes the changelog entry

Copy link
Owner

Choose a reason for hiding this comment

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

Great work!

I just can't check all upstream changes, but if CI is happy and you checked it, I'm more than happy!

On deleting the new modules... Once the commit is there, it won't save disk/repo space unless force pushes are used, so I tend to think it's better to just live with them for now.

Copy link
Owner

@jlblancoc jlblancoc left a comment

Choose a reason for hiding this comment

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

Great work!

I just can't check all upstream changes, but if CI is happy and you checked it, I'm more than happy!

On deleting the new modules... Once the commit is there, it won't save disk/repo space unless force pushes are used, so I tend to think it's better to just live with them for now.

@jlblancoc jlblancoc merged commit 1e07daa into master Jan 22, 2024
10 checks passed
@NeroBurner NeroBurner deleted the suitesparse_7.5.1 branch January 23, 2024 07:14
@NeroBurner
Copy link
Collaborator Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Request for a new feature or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants