Skip to content

feat: Suggestions for the unified platform#282

Open
mhovd wants to merge 33 commits into
unified-platform-structurefrom
unified-suggestions
Open

feat: Suggestions for the unified platform#282
mhovd wants to merge 33 commits into
unified-platform-structurefrom
unified-suggestions

Conversation

@mhovd
Copy link
Copy Markdown
Collaborator

@mhovd mhovd commented May 25, 2026

No description provided.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 25, 2026

🐰 Bencher Report

Branchunified-suggestions
Testbedmhovd-pgx

🚨 1 Alert

BenchmarkMeasure
Units
ViewBenchmark Result
(Result Δ%)
Upper Boundary
(Limit %)
bimodal_ke_npagLatency
seconds (s)
📈 plot
🚷 threshold
🚨 alert (🔔)
12.09 s
(+157.30%)Baseline: 4.70 s
6.39 s
(189.09%)

Click to view all benchmark results
BenchmarkLatencyBenchmark Result
milliseconds (ms)
(Result Δ%)
Upper Boundary
milliseconds (ms)
(Limit %)
bimodal_ke_npag📈 view plot
🚷 view threshold
🚨 view alert (🔔)
12,088.00 ms
(+157.30%)Baseline: 4,698.06 ms
6,392.73 ms
(189.09%)

bimodal_ke_npod📈 view plot
🚷 view threshold
1,516.20 ms
(+13.64%)Baseline: 1,334.20 ms
1,675.54 ms
(90.49%)
bimodal_ke_postprob📈 view plot
🚷 view threshold
522.31 ms
(+110.76%)Baseline: 247.82 ms
722.61 ms
(72.28%)
🐰 View full continuous benchmarking report in Bencher

@mhovd mhovd changed the base branch from main to unified-platform-structure May 26, 2026 11:39
@mhovd mhovd marked this pull request as ready for review May 28, 2026 18:42
@mhovd mhovd changed the title wip: Suggestions for the unified platform feat: Suggestions for the unified platform May 28, 2026
Comment thread examples/drusano/main.rs
.parameter(Parameter::bounded("h2r2", 10.0, 22.0))?
.method(Npag::new())
.nonparametric()
.parameter(BoundedParameter::new("v1", 5.0, 160.0))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I kinda prefer Parqameter::bounded() to BoundedParameter::new() as rust-analyzer's auto-completion will be a bit more helpful with the former.

Not very important tbh, as autocompletion will also trigger when writting Parameter and show all options.

Comment thread examples/drusano/main.rs
.error(
"outeq_1",
AssayErrorModel::proportional(ErrorPoly::new(0.1, 0.1, 0.0, 0.0), 1.0),
)?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think .error() should return an error. If the user provides an outeq not listed in the model's metadata it should fail while constructing the problem and not during execuiton

Comment thread examples/meta/main.rs
.unwrap()
.method(Npod::new())
.nonparametric()
.parameter(BoundedParameter::new("cls", 0.1, 10.0))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is the same with parameter. I think it should return error, so we can detect errors in the name of the ranges when constructing the problem and not during execution

Comment thread examples/bestdose.rs
let parameter_space = ParameterSpace::new()
.add(Parameter::bounded("ke", 0.001, 3.0))
.add(Parameter::bounded("v", 25.0, 250.0));
let parameter_space = NonParametricParameters::new()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

NonParametricParameters is a fun word. Do we need a distinction between parametric and non parametric parameter spaces?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Actually no, I can change it to be generic, which is more flexible and easier to use. Good suggestion!

Comment thread examples/bestdose_auc.rs
println!("Loading prior from bimodal_ke example...");
let (theta, prior) = read_prior("examples/bimodal_ke/output/theta.csv", &parameter_space)?;
let (theta, prior) = Theta::from_file(
&"examples/bimodal_ke/output/theta.csv".to_string(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like this

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.

2 participants