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

Fix Compatibility issues (fix Pipeline) #48

Merged
merged 88 commits into from
Feb 26, 2025
Merged

Fix Compatibility issues (fix Pipeline) #48

merged 88 commits into from
Feb 26, 2025

Conversation

absternator
Copy link
Contributor

@absternator absternator commented Feb 26, 2025

The current version 1.2.3 deployed falls over when running on Python >=v3.12 and The CXX compiler GNU) version >=13. Thus the following fixes have been made to ensure it works across these permutations of dependencies. These are both used by conda-forge in the feedstock and therefore should fix current feedstock pipeline woes.

Python >3.12 issues

  • in sce.py line 34, pd.Series.to_csv uses sep='\n' which is no longer valid, and thus is removed as this is already the default for the series
  • segmentation fault in pairsnp.hpp when checking PyErr_CheckSignals() inside of openmp loop. To fix, Had to ensure this is only checked from the main thread.

GNU 13 issues

  • FORTIFY_SOURCE raising compilation errors inside downloaded boost library in /vendor folder. This is due to new compiler version being more strict with these older files. Thus have reverted PR Vendor boost lib #33, so that cmake finds Boost once again. (Have spoken to Rich who created the PR about this)
  • conditional_probabilities func in wtsne.hpp was returning 0s and nans causing failure later in "RuntimeError: Probability vector has negative entries" in line 22 of vector_norm.hpp. The reason was because std::numeric_limits<real_t>::infinity() was used for beta_min and beta_max. This infinity causes issues with different compiler versions and thus have changed to use std::numeric_limits<real_t>::max().

Also small type mismatch int & size_t in kseq.h1 that raised warnings. Thus have typecasted to fix.

Note: ubuntu-22.04 uses GNU 11 and ubuntu-latest uses GNU-13. Thus have added as matrix in pipeline

…Python components in find_package; update pybind11 version in environment.yml
…remove unused C++ standard setting in CMakeLists.txt
…nclude directory handling"

This reverts commit e40625d.
Remove wchar.h include from wtsne.hpp to clean up unused dependencies
…gs; add debug output in normalise_vector function
… cwchar in python_bindings.cpp for improved compatibility and standard compliance
…a values and optimize loop indexing; add debug output for P values
…variables with simpler types and optimizing OpenMP scheduling
@absternator absternator requested a review from johnlees February 26, 2025 08:51
@johnlees johnlees linked an issue Feb 26, 2025 that may be closed by this pull request
Copy link
Member

@johnlees johnlees left a comment

Choose a reason for hiding this comment

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

Thanks for this mammoth effort!! All these changes make sense

Sorry about the openmp pyerrcheck one – I'd found that and fixed it in pp-sketchlib after some pain but had forgotten the same was being done here

@johnlees johnlees merged commit 4354922 into master Feb 26, 2025
5 checks passed
@johnlees johnlees deleted the fix-pipepines branch February 26, 2025 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compilation issue. Do not provide vendor directory nor header
2 participants