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

remake fails silently for vector-valued parameters #3239

Open
ysfoo opened this issue Nov 26, 2024 · 3 comments
Open

remake fails silently for vector-valued parameters #3239

ysfoo opened this issue Nov 26, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@ysfoo
Copy link

ysfoo commented Nov 26, 2024

remake does not update parameters that are vector-valued. The following example is taken from the Catalyst documentation:

using OrdinaryDiffEq
using Catalyst

two_state_model = @reaction_network begin    
    @parameters k[1:2]
    @species X(t)[1:2]
    (k[1],k[2]), X[1] <--> X[2]
end

u0 = [:X => [0.0, 2.0]];
tspan = (0.0, 1.0);

ps = [:k => [1.0, 2.0]];
oprob = ODEProblem(two_state_model, u0, tspan, ps);
oprob_rmk = remake(oprob; p=[:k => [0.0, 0.0]]);

oprob_rmk does not have the updated parameters; both solve(oprob)(1.0) and solve(oprob_rmk)(1.0) produce the same output.

Interestingly, this behaviour is absent if the parameters are defined outside of the reaction_network macro:

@parameters k[1:2]
two_state_model = @reaction_network begin    
    @species X(t)[1:2]
    ($k[1],$k[2]), X[1] <--> X[2]
end

u0 = [:X => [0.0, 2.0]];
tspan = (0.0, 1.0);

ps = [k => [1.0, 2.0]];
oprob = ODEProblem(two_state_model, u0, tspan, ps);
oprob_rmk = remake(oprob; p=[k => [0.0, 0.0]]);

Here, solve(oprob_rmk)(1.0) produces the expected output, i.e. [0.0, 2.0].

@ysfoo ysfoo added the bug Something isn't working label Nov 26, 2024
@AayushSabharwal
Copy link
Member

This doesn't work because Catalyst scalarizes vector parameters declared in the @reaction_network macro. As a result, MTK considers each of the elements as separate parameters. I can get it to recognize the vector version of k in limited cases, but @TorkelE would it be possible for Catalyst to not scalarize here?

@ChrisRackauckas
Copy link
Member

Ahh that makes sense. The scalarizing can also potentially inhibit some of the codegen optimizations that are starting to happen as well, so it would be best to solve this by not scalarizing in Catalyst.

@TorkelE
Copy link
Member

TorkelE commented Jan 6, 2025

I think this might be fine on Catalyst master, but will need to check it tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants