Skip to content

Conversation

@ahnaf-tahmid-chowdhury
Copy link
Contributor

@ahnaf-tahmid-chowdhury ahnaf-tahmid-chowdhury commented Mar 6, 2025

Description

This PR introduces support for scikit-build-core, streamlining the build process and improving compatibility.

This is a minimal version of the PR, excluding manylinux support, to facilitate a faster review. The full PR can be found here.

Key Changes

Enable scikit-build-core support

  • Updated pyproject.toml to use scikit-build-core instead of legacy methods.

CMake and Build Updates

  • Updated CMakeLists.txt to support skbuild, ensuring compatibility with both scikit-build-core and native CMake builds.
  • Added GenerateScript.cmake to handle executable files (openmc) on both Linux and macOS.
  • Updated OpenMCConfig.cmake.in to integrate scikit-build-core.

Git and Packaging Improvements

  • Removed ignoring of .h5 files from .gitignore as they are needed.
  • Removed MANIFEST.in as it is no longer required.

CI and Docs Updates

  • Updated Dockerfile: Removed the need to run CMake before pip install as scikit-build-core now handles it.
  • Updated CI configuration files to fix build and test issues.
  • Added necessary APT packages for ReadTheDocs to compile dependencies properly.

Python API Enhancements

  • Added functions to __init__.py to retrieve OpenMC installation configuration from the Python side.

Sponsored By


This contribution is kindly sponsored by @proximafusion, supporting the ongoing development and modernization of OpenMC’s Python packaging and build system.

@ahnaf-tahmid-chowdhury
Copy link
Contributor Author

We need to do the same thing for mcpl that we have done for ncrystal.

@ahnaf-tahmid-chowdhury
Copy link
Contributor Author

I have already created PR #3429 to make this PR working again.

@ahnaf-tahmid-chowdhury
Copy link
Contributor Author

Everything is working now.

@nplinden
Copy link
Contributor

nplinden commented Aug 4, 2025

Here's a minimal Dockerfile I used to produce the wheel for python 3.13/ubuntu25.04, works great:

FROM ubuntu:25.04

RUN apt update -y \
    && apt install -y python3 python-is-python3 python3-venv python3-dev \
                      git g++ cmake libhdf5-dev libpng-dev curl

RUN python -m venv venv \
    && . venv/bin/activate \
    && pip install git+https://github.com/ahnaf-tahmid-chowdhury/OpenMC.git@skbuild-minimal

What are the difficulties that need solving to add DAGMC+dependencies to the wheel?

@shimwell
Copy link
Member

shimwell commented Aug 4, 2025

Glad to hear it is working for you.

What are the difficulties that need solving to add DAGMC+dependencies to the wheel?

No difficulties, Ahnaf and myself have already added DAGMC + Embree + Moab to a wheel. We have not included it in this PR as we are trying to keep it minimal. If this one is merged then we would follow up with a PR that adds DAGMC to the wheel. However for a sneak preview you could take a peak that this branch and this repo for method and resulting wheels. One difference is that we have used manylinux dockerfiles to ensure compatibility with other flavors of linux.

@shimwell
Copy link
Member

Ah looks like the recent changes to the coverage need merging into this PR

CI error is
Run # Merge C++ and Python LCOV into a single file for upload
cat: coverage-cpp.lcov: No such file or directory

I think this is the PR that got the CI working again #3594

@ahnaf-tahmid-chowdhury
Copy link
Contributor Author

Key things being changed, added, or removed:

  • Build System Update: The pyproject.toml file is updated to use scikit-build-core as the build backend.
  • CMake Build Updates:
    • CMakeLists.txt and related files have been adapted to support skbuild, ensuring compatibility with both scikit-build-core and regular CMake builds.
    • A new GenerateScript.cmake is added for handling executable files on Linux and macOS.
    • OpenMCConfig.cmake.in updated for integration with the new build system.
  • Packaging & Project Files:
    • .gitignore is updated to stop ignoring .h5 files, as they are now required.
    • MANIFEST.in has been removed since it's no longer necessary.
  • Continuous Integration & Documentation:
    • The Dockerfile is updated to give scikit-build-core responsibility for CMake steps.
    • Continuous integration and documentation build configs are updated to fix issues and ensure all dependencies are built properly.
    • Additional APT packages are added for ReadTheDocs builds.
  • Python API Enhancements: Functions added to __init__.py to dynamically retrieve installation configuration details from Python.
  • General Modernization: Old methods are deprecated/removed in favor of newer, streamlined approaches.

@ahnaf-tahmid-chowdhury
Copy link
Contributor Author

This is just the foundation. More features will be added later, such as using CMake’s built-in modules for target generation, adding an openmc-config CLI, supporting manylinux images for PyPI, and improving the handling of external packages.

Copy link
Contributor

@GuySten GuySten left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@GuySten GuySten added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Dec 17, 2025
Copy link
Contributor

@paulromano paulromano left a comment

Choose a reason for hiding this comment

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

Putting a placeholder as I'd like to review this before we merge

@GuySten GuySten removed the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Dec 17, 2025
Copy link
Contributor

@GuySten GuySten left a comment

Choose a reason for hiding this comment

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

I noticed that the cpp test suite does not run.

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.

7 participants