chore: Log likelihood suggestions

[mhovd]

No description provided.

[Copilot]

Pull request overview

This PR refactors the likelihood matrix representation by replacing a boolean flag is_log_space with a more type-safe Space enum that can be either Linear or Log. This improves code clarity and maintainability by making the distinction between linear and log space explicit in the type system.

Key Changes:

• Introduced a Space enum with Linear and Log variants to replace the boolean is_log_space field
• Refactored calculate_psi_dispatch, calculate_psi, and calculate_log_psi into a single calculate_psi function that takes a Space parameter
• Removed the PsiBuilder helper struct in favor of direct new_linear and new_log constructors
• Updated all algorithm implementations (NPAG, NPOD, POSTPROB) and utility functions to use the new Space enum

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/structs/psi.rs	Added Space enum, refactored Psi struct to use it, consolidated likelihood calculation functions, removed PsiBuilder
src/routines/settings.rs	Updated Advanced settings struct to use Space enum instead of boolean
src/routines/output/posterior.rs	Converted Space enum to boolean for existing logic
src/routines/estimation/qr.rs	Updated to use match expression on Space enum
src/routines/estimation/ipm.rs	Updated tests to use new new_log constructor, removed PsiBuilder references
src/bestdose/posterior.rs	Updated function signature to use Space parameter instead of boolean
src/algorithms/postprob.rs	Updated to call unified calculate_psi with Space parameter
src/algorithms/npod.rs	Updated to call unified calculate_psi with Space parameter, converted to use match on Space
src/algorithms/npag.rs	Updated to call unified calculate_psi with Space parameter
src/algorithms/mod.rs	Converted Space enum to boolean for existing validation logic

Comments suppressed due to low confidence (1)

src/algorithms/npag.rs:180

• For consistency with other parts of the codebase (see src/routines/estimation/qr.rs, src/algorithms/npod.rs, src/routines/estimation/ipm.rs), consider using a match expression on psi.space() instead of an if-else comparison. This makes the code more consistent and explicitly handles both Space variants.

                self.f1 = if self.psi.space() == crate::structs::psi::Space::Log {
                    // For log-space: f1 = sum_i(logsumexp(log_psi[i,:] + log(w)))
                    let log_w: Vec<f64> = w.weights().iter().map(|&x| x.ln()).collect();
                    (0..psi.nrows())
                        .map(|i| {
                            let combined: Vec<f64> = (0..psi.ncols())
                                .map(|j| *psi.get(i, j) + log_w[j])
                                .collect();
                            logsumexp(&combined)
                        })
                        .sum()
                } else {
                    // For regular space: f1 = sum(log(psi * w))
                    let pyl = psi * w.weights();
                    pyl.iter().map(|x| x.ln()).sum()
                };

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The documentation comment says "Create a new Psi in log space" but this function creates a Psi in linear space. The comment should say "Create a new Psi in linear space" instead.

Suggested change

	/// Create a new Psi in log space
	/// Create a new Psi in linear space

[Copilot]

The set_space method allows changing the Space field without actually converting the underlying matrix data. This could lead to incorrect results if the matrix values represent linear likelihoods but the space is set to Log, or vice versa. Consider either removing this method or documenting that it should only be used when the caller has already transformed the matrix values appropriately.

Suggested change

	/// Set the [Space] (Linear or Log) of the Psi matrix
	pub fn set_space(&mut self, space: Space) {
	self.space = space;
	}
	// Removed set_space to prevent changing space without transforming matrix data.

[Copilot]

The matrix dimensions are swapped. The Mat::from_fn call creates a matrix with shape (n_point, n_sub), but it should be (n_sub, n_point) to match the original regular_mat and the expected format for burke_log. The indices in the lambda should also be swapped to (j, i) to correctly map from log_mat which has shape (n_sub, n_point).

Suggested change

	let mat = Mat::from_fn(n_point, n_sub, |i, j| log_mat[(i, j)]);
	let mat = Mat::from_fn(n_sub, n_point, |i, j| log_mat[(i, j)]);

[Copilot]

The matrix dimensions are swapped. The Mat::from_fn call creates a matrix with shape (n_point, n_sub), but it should be (n_sub, n_point) to match the expected format for burke_log. The indices in the lambda should also be swapped to (j, i) to correctly map from log_mat which has shape (n_sub, n_point).

Suggested change

	let mat = Mat::from_fn(n_point, n_sub, |i, j| log_mat[(i, j)]);
	let mat = Mat::from_fn(n_sub, n_point, |i, j| log_mat[(i, j)]);

[github-actions]

Bencher Report

Branch	log-likelihood-suggestions
Testbed	rust-moan

⚠️ WARNING: No Threshold found!

Without a Threshold, no Alerts will ever be generated.

• Latency (nanoseconds (ns))

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds flag.

Click to view all benchmark results

Benchmark	Latency	seconds (s)
bimodal_ke_npag	📈 view plot ⚠️ NO THRESHOLD	13.74 s
bimodal_ke_npod	📈 view plot ⚠️ NO THRESHOLD	5.79 s
bimodal_ke_postprob	📈 view plot ⚠️ NO THRESHOLD	1.70 s

🐰 View full continuous benchmarking report in Bencher