fix: only evaluate required keys of varmap in process_SciMLProblem

[AayushSabharwal]

Close #3153

Checklist

• Appropriate tests were added
• Any code changes were done in a way that does not break public API
• All documentation related to code changes were updated
• The new code follows the
  contributor guidelines, in particular the SciML Style Guide and
  COLPRAC.
• Any new documentation only uses public API

Additional context

Add any other context about the problem here.

[isaacsas]

Has this been tested against catalyst to ensure it won’t break stuff? Note that CI here is not currently doing such a test.

[isaacsas]

(Because of BifurcationKit not getting updated yet.)

[ChrisRackauckas]

Why wouldn't the downstream test be sufficient on all?

Also, Catalyst doesn't use this function at all https://github.com/search?q=repo%3ASciML%2FCatalyst.jl%20evaluate_varmap!&type=code

[AayushSabharwal]

Additionally, this was the behavior of the *Problem constructors before the cleanup anyway. I changed to evaluating the entire varmap because it seemed cleaner to do it once and use the "finalized" varmap everywhere but didn't know of the edge case in the linked issue.

[AayushSabharwal]

I'll test master against catalyst, though, because of #3155

[isaacsas]

@ChrisRackauckas Just look at the integration test on Catalyst here — it didn’t actually run due to the bifurcation kit version incompatibility.

I will remove that extension from Catalyst later today until someone can update it (to get downstream testing here working again). More generally though, this issue is part of why I wanted to move extensions to separate libs.

[isaacsas]

@AayushSabharwal ok, thanks for explaining. Just didn’t want to break the conservation law handling in problem construction suddenly. If this was the old behavior it should be fine.

[AayushSabharwal]

I can't get Catalyst tests to run. Artificially lowering BifurcationKit compat doesn't work, and neither does removing the extension altogether.

[isaacsas]

Ok, I’ll see what is going on when I’m at work later, and get Catalyst master working with downstream here again. I’m not sure why removing BifurcationKit from the Project.toml and testing isn’t sufficient.

[ChrisRackauckas]

Okay, since Catalyst does not use this function and it's reverting to an older behavior, I'm going to merge. I'm confused why Catalyst is brought up here in the first place, though we should fix that extension, I do not understand why the concern on a function it doesn't use?

[isaacsas]

Every *Problem is built via that function. I was concerned changing the initialization in this way could break something we rely on (like conserved constant initialization). if it is the old behavior it should be fine.