Clean up PR review issues: remove hardcoded paths, deduplicate unified_sim, optimize CI

Author: ciaranra

.github/workflows/python-release.yml

@@ -297,10 +297,13 @@ jobs:
             LLVM_SYS_140_PREFIX=$HOME/.pecos/deps/llvm
             MACOSX_DEPLOYMENT_TARGET=13.2
           CIBW_BEFORE_ALL_MACOS: |
-            curl -sSf https://sh.rustup.rs | sh -s -- -y
-            source $HOME/.cargo/env
-            rustup update
-            cargo run --release -p pecos --features cli -- install llvm --force
+            source $HOME/.cargo/env 2>/dev/null || { curl -sSf https://sh.rustup.rs | sh -s -- -y && source $HOME/.cargo/env; }
+            if [ ! -d "$HOME/.pecos/deps/llvm/bin" ]; then
+              rustup update
+              cargo run --release -p pecos --features cli -- install llvm --force
+            else
+              echo "LLVM already installed from pecos-rslib build, skipping"
+            fi
             mkdir -p $HOME/.pecos/bin
             printf '#!/bin/bash\nunset DYLD_LIBRARY_PATH\nexec /usr/bin/codesign "$@"\n' > $HOME/.pecos/bin/codesign
             chmod +x $HOME/.pecos/bin/codesign
@@ -312,7 +315,7 @@ jobs:
             LLVM_SYS_140_PREFIX="C:\\Users\\runneradmin\\.pecos\\deps\\llvm"
           CIBW_BEFORE_ALL_WINDOWS: >
             rustup update &&
-            cargo run --release -p pecos --features cli -- install llvm --force
+            if not exist "C:\Users\runneradmin\.pecos\deps\llvm\bin" (cargo run --release -p pecos --features cli -- install llvm --force) else (echo LLVM already installed from pecos-rslib build)
           CIBW_BEFORE_BUILD_WINDOWS: >
             pip install delvewheel &&
             python -c "import delvewheel._dll_list as d,inspect,re as r;p=inspect.getfile(d);c=open(p).read();n=chr(10);open(p,'w').write(c.replace(r\"re.compile('api-.*'),\",r\"re.compile('api-.*'),\"+n+r\"    re.compile('ext-.*'),\")) if 'ext-.*' not in c else None"

.github/workflows/rust-test.yml

@@ -14,6 +14,7 @@ on:
       - 'examples/**'
       - 'julia/pecos-julia-ffi/**'
       - 'python/pecos-rslib-cuda/**'
+      - 'python/pecos-rslib/**'
       - 'python/pecos-rslib-llvm/**'
       - 'Cargo.toml'
       - '.github/workflows/rust-test.yml'
@@ -25,6 +26,7 @@ on:
       - 'examples/**'
       - 'julia/pecos-julia-ffi/**'
       - 'python/pecos-rslib-cuda/**'
+      - 'python/pecos-rslib/**'
       - 'python/pecos-rslib-llvm/**'
       - 'Cargo.toml'
       - '.github/workflows/rust-test.yml'

crates/pecos/src/unified_sim.rs

@@ -8,7 +8,22 @@ use pecos_engines::{ClassicalControlEngineBuilder, MonteCarloEngine, SimBuilder,
 use pecos_programs::Program;
 use pecos_qasm::qasm_engine;
 #[cfg(feature = "qis")]
-use pecos_qis::qis_engine;
+use pecos_qis::{IntoQisInterface, qis_engine};
+
+/// Set up a QIS engine with Selene runtime and Helios interface for the given program.
+#[cfg(feature = "qis")]
+fn build_qis_engine<P: IntoQisInterface + 'static>(
+    program: P,
+) -> Result<pecos_qis::QisEngineBuilder, PecosError> {
+    let selene_runtime = crate::selene_simple_runtime()
+        .map_err(|e| PecosError::Generic(format!("Failed to load Selene runtime: {e}")))?;
+    let helios_builder = crate::helios_interface_builder();
+    qis_engine()
+        .runtime(selene_runtime)
+        .interface(helios_builder)
+        .try_program(program)
+        .map_err(|e| PecosError::Generic(format!("Failed to load program: {e}")))
+}
 
 /// Extension trait for `SimBuilder` to add program-based methods
 pub trait SimBuilderExt {
@@ -43,184 +58,77 @@ pub struct ProgrammedSimBuilder {
 }
 
 impl ProgrammedSimBuilder {
+    /// Auto-select the classical engine based on program type, returning a configured `SimBuilder`.
+    fn configure_engine(self) -> Result<SimBuilder, PecosError> {
+        if self.override_classical {
+            return Ok(self.base_builder);
+        }
+
+        match self.program {
+            Program::Qasm(qasm) => Ok(self.base_builder.classical(qasm_engine().program(qasm))),
+            Program::Qis(qis) => {
+                #[cfg(feature = "qis")]
+                {
+                    let engine_builder = build_qis_engine(qis)?;
+                    Ok(self.base_builder.classical(engine_builder))
+                }
+                #[cfg(not(feature = "qis"))]
+                {
+                    let _ = qis;
+                    Err(PecosError::Generic(
+                        "QIS programs require Selene and LLVM support. Please rebuild with --features selene,llvm".to_string()
+                    ))
+                }
+            }
+            Program::Hugr(hugr) => {
+                #[cfg(feature = "qis")]
+                {
+                    let engine_builder = build_qis_engine(hugr)?;
+                    Ok(self.base_builder.classical(engine_builder))
+                }
+                #[cfg(not(feature = "qis"))]
+                {
+                    let _ = hugr;
+                    Err(PecosError::Generic(
+                        "HUGR programs require Selene and LLVM support. Please rebuild with --features selene,llvm".to_string()
+                    ))
+                }
+            }
+            Program::Wasm(_) => Err(PecosError::Input(
+                "WASM programs are not yet supported in unified simulation".to_string(),
+            )),
+            Program::Wat(_) => Err(PecosError::Input(
+                "WAT programs are not yet supported in unified simulation".to_string(),
+            )),
+            Program::PhirJson(_) => Err(PecosError::Input(
+                "PHIR JSON programs are not yet supported in unified simulation".to_string(),
+            )),
+            Program::SeleneInterface(_) => Err(PecosError::Input(
+                "SeleneInterface programs are not yet supported in unified simulation".to_string(),
+            )),
+        }
+    }
+
     /// Build the simulation with automatic engine selection
     ///
-    /// This selects an engine based on the program type and builds the simulation,
-    /// unless a classical engine was already explicitly set.
-    ///
     /// # Errors
     ///
     /// Returns an error if:
     /// - The program type is not yet supported (WASM, WAT, PHIR JSON, `SeleneInterface`)
     /// - Engine building fails
     pub fn build(self) -> Result<MonteCarloEngine, PecosError> {
-        if self.override_classical {
-            // Classical engine was already set, just build
-            self.base_builder.build()
-        } else {
-            // Auto-select engine based on program type
-            match self.program {
-                Program::Qasm(qasm) => self
-                    .base_builder
-                    .classical(qasm_engine().program(qasm))
-                    .build(),
-                Program::Qis(qis) => {
-                    // Use Selene runtime and Helios interface
-                    #[cfg(feature = "qis")]
-                    {
-                        let selene_runtime = crate::selene_simple_runtime().map_err(|e| {
-                            PecosError::Generic(format!("Failed to load Selene runtime: {e}"))
-                        })?;
-                        let helios_builder = crate::helios_interface_builder();
-                        let engine_builder = qis_engine()
-                            .runtime(selene_runtime)
-                            .interface(helios_builder)
-                            .try_program(qis)
-                            .map_err(|e| {
-                                PecosError::Generic(format!("Failed to load QIS program: {e}"))
-                            })?;
-
-                        self.base_builder.classical(engine_builder).build()
-                    }
-                    #[cfg(not(feature = "qis"))]
-                    {
-                        let _ = qis; // Mark as used to avoid warning
-                        Err(PecosError::Generic(
-                            "QIS programs require Selene and LLVM support. Please rebuild with --features selene,llvm".to_string()
-                        ))
-                    }
-                }
-                Program::Hugr(hugr) => {
-                    // Use Selene runtime and Helios interface for HUGR programs
-                    #[cfg(feature = "qis")]
-                    {
-                        let selene_runtime = crate::selene_simple_runtime().map_err(|e| {
-                            PecosError::Generic(format!("Failed to load Selene runtime: {e}"))
-                        })?;
-                        let helios_builder = crate::helios_interface_builder();
-                        let engine_builder = qis_engine()
-                            .runtime(selene_runtime)
-                            .interface(helios_builder)
-                            .try_program(hugr)
-                            .map_err(|e| {
-                                PecosError::Generic(format!("Failed to load HUGR program: {e}"))
-                            })?;
-
-                        self.base_builder.classical(engine_builder).build()
-                    }
-                    #[cfg(not(feature = "qis"))]
-                    {
-                        let _ = hugr; // Mark as used to avoid warning
-                        Err(PecosError::Generic(
-                            "HUGR programs require Selene and LLVM support. Please rebuild with --features selene,llvm".to_string()
-                        ))
-                    }
-                }
-                Program::Wasm(_) => Err(PecosError::Input(
-                    "WASM programs are not yet supported in unified simulation".to_string(),
-                )),
-                Program::Wat(_) => Err(PecosError::Input(
-                    "WAT programs are not yet supported in unified simulation".to_string(),
-                )),
-                Program::PhirJson(_) => Err(PecosError::Input(
-                    "PHIR JSON programs are not yet supported in unified simulation".to_string(),
-                )),
-                Program::SeleneInterface(_) => Err(PecosError::Input(
-                    "SeleneInterface programs are not yet supported in unified simulation"
-                        .to_string(),
-                )),
-            }
-        }
+        self.configure_engine()?.build()
     }
 
     /// Build and run the simulation with automatic engine selection
     ///
-    /// This selects an engine based on the program type and runs the simulation,
-    /// unless a classical engine was already explicitly set.
-    ///
     /// # Errors
     ///
     /// Returns an error if:
     /// - The program type is not yet supported (WASM, WAT, PHIR JSON, `SeleneInterface`)
     /// - Engine building or running fails
     pub fn run(self, shots: usize) -> Result<pecos_engines::shot_results::ShotVec, PecosError> {
-        if self.override_classical {
-            // Classical engine was already set, just run
-            self.base_builder.run(shots)
-        } else {
-            // Auto-select engine based on program type
-            match self.program {
-                Program::Qasm(qasm) => self
-                    .base_builder
-                    .classical(qasm_engine().program(qasm))
-                    .run(shots),
-                Program::Qis(qis) => {
-                    // Use Selene runtime and Helios interface
-                    #[cfg(feature = "qis")]
-                    {
-                        let selene_runtime = crate::selene_simple_runtime().map_err(|e| {
-                            PecosError::Generic(format!("Failed to load Selene runtime: {e}"))
-                        })?;
-                        let helios_builder = crate::helios_interface_builder();
-                        let engine_builder = qis_engine()
-                            .runtime(selene_runtime)
-                            .interface(helios_builder)
-                            .try_program(qis)
-                            .map_err(|e| {
-                                PecosError::Generic(format!("Failed to load QIS program: {e}"))
-                            })?;
-
-                        self.base_builder.classical(engine_builder).run(shots)
-                    }
-                    #[cfg(not(feature = "qis"))]
-                    {
-                        let _ = qis; // Mark as used to avoid warning
-                        Err(PecosError::Generic(
-                            "QIS programs require Selene and LLVM support. Please rebuild with --features selene,llvm".to_string()
-                        ))
-                    }
-                }
-                Program::Hugr(hugr) => {
-                    // Use Selene runtime and Helios interface for HUGR programs
-                    #[cfg(feature = "qis")]
-                    {
-                        let selene_runtime = crate::selene_simple_runtime().map_err(|e| {
-                            PecosError::Generic(format!("Failed to load Selene runtime: {e}"))
-                        })?;
-                        let helios_builder = crate::helios_interface_builder();
-                        let engine_builder = qis_engine()
-                            .runtime(selene_runtime)
-                            .interface(helios_builder)
-                            .try_program(hugr)
-                            .map_err(|e| {
-                                PecosError::Generic(format!("Failed to load HUGR program: {e}"))
-                            })?;
-
-                        self.base_builder.classical(engine_builder).run(shots)
-                    }
-                    #[cfg(not(feature = "qis"))]
-                    {
-                        let _ = hugr; // Mark as used to avoid warning
-                        Err(PecosError::Generic(
-                            "HUGR programs require Selene and LLVM support. Please rebuild with --features selene,llvm".to_string()
-                        ))
-                    }
-                }
-                Program::Wasm(_) => Err(PecosError::Input(
-                    "WASM programs are not yet supported in unified simulation".to_string(),
-                )),
-                Program::Wat(_) => Err(PecosError::Input(
-                    "WAT programs are not yet supported in unified simulation".to_string(),
-                )),
-                Program::PhirJson(_) => Err(PecosError::Input(
-                    "PHIR JSON programs are not yet supported in unified simulation".to_string(),
-                )),
-                Program::SeleneInterface(_) => Err(PecosError::Input(
-                    "SeleneInterface programs are not yet supported in unified simulation"
-                        .to_string(),
-                )),
-            }
-        }
+        self.configure_engine()?.run(shots)
     }
 
     /// Override the classical engine selection

python/pecos-rslib/src/lib.rs

@@ -173,32 +173,42 @@ fn pecos_rslib(_py: Python<'_>, m: &Bound<'_, PyModule>) -> PyResult<()> {
 
         log::debug!("Unix detected, attempting Selene runtime preload...");
 
-        // Try to find libselene_simple_runtime.so
-        let possible_paths = [
-            "/home/ciaranra/Repos/cl_projects/gup/selene/target/debug/libselene_simple_runtime.so",
-            "/home/ciaranra/Repos/cl_projects/gup/selene/target/release/libselene_simple_runtime.so",
-            "../selene/target/debug/libselene_simple_runtime.so",
-            "../selene/target/release/libselene_simple_runtime.so",
-        ];
+        // Build search paths for libselene_simple_runtime.so:
+        // 1. PECOS_SELENE_PRELOAD env var (explicit override)
+        // 2. ~/.pecos/lib/
+        // 3. Relative development paths (target/debug, target/release)
+        let mut possible_paths: Vec<std::path::PathBuf> = Vec::new();
+
+        if let Ok(path) = std::env::var("PECOS_SELENE_PRELOAD") {
+            possible_paths.push(std::path::PathBuf::from(path));
+        }
+
+        if let Some(home) = dirs::home_dir() {
+            possible_paths.push(home.join(".pecos/lib/libselene_simple_runtime.so"));
+        }
+
+        possible_paths.push("target/debug/libselene_simple_runtime.so".into());
+        possible_paths.push("target/release/libselene_simple_runtime.so".into());
 
         log::debug!("Checking for Selene runtime libraries...");
         for path in &possible_paths {
-            log::trace!("Checking path: {path}");
-            if std::path::Path::new(path).exists() {
-                log::debug!("Found Selene runtime! Attempting to preload: {path}");
+            let path_str = path.to_string_lossy();
+            log::trace!("Checking path: {path_str}");
+            if path.exists() {
+                log::debug!("Found Selene runtime! Attempting to preload: {path_str}");
 
                 unsafe {
-                    let path_cstr = CString::new(path.as_bytes()).unwrap();
+                    let path_cstr = CString::new(path_str.as_bytes()).unwrap();
                     let handle = libc::dlopen(path_cstr.as_ptr(), RTLD_LAZY | RTLD_GLOBAL);
                     if handle.is_null() {
                         let error_ptr = libc::dlerror();
                         if !error_ptr.is_null() {
                             let error = std::ffi::CStr::from_ptr(error_ptr).to_string_lossy();
-                            log::warn!("Failed to preload {path}: {error}");
+                            log::warn!("Failed to preload {path_str}: {error}");
                         }
                     } else {
                         log::info!(
-                            "Successfully preloaded Selene runtime with RTLD_GLOBAL from: {path}"
+                            "Successfully preloaded Selene runtime with RTLD_GLOBAL from: {path_str}"
                         );
                         break;
                     }