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

Bump Bio for Catalyst v14 and MTK v9 #1022

Merged
merged 9 commits into from
Nov 19, 2024
Merged

Bump Bio for Catalyst v14 and MTK v9 #1022

merged 9 commits into from
Nov 19, 2024

Conversation

ChrisRackauckas
Copy link
Member

No description provided.

@ChrisRackauckas
Copy link
Member Author

@TorkelE @isaacsas this is the one to watch for the major FBDF changes

@isaacsas
Copy link
Member

I’d imagine many of these need updating for Catalyst 14 (eg ReactionNetworkImorters generated systems need to be marked complete).

@ChrisRackauckas
Copy link
Member Author

@isaacsas can you help with what needs to be changed with this bump?

@isaacsas
Copy link
Member

@ChrisRackauckas LMK if this doesn't work. I think all that was needed was to complete the converted ODESystem, and get the u0 and p from the oprob now (since varmap_to_vars no longer gives the right format for p).

On a sidenote, it would be useful if the parameter to MTKParameter conversion functionality could be factored into an exported user-accessible function so users can manually process parameter maps to the new form. (Essentially a varmap_to_vars replacement.)

@ChrisRackauckas
Copy link
Member Author

On a sidenote, it would be useful if the parameter to MTKParameter conversion functionality could be factored into an exported user-accessible function so users can manually process parameter maps to the new form. (Essentially a varmap_to_vars replacement.)

Interesting point. Open an issue.

@avik-pal
Copy link
Member

How long is this benchmark? It seems to error https://buildkite.com/julialang/scimlbenchmarks-dot-jl/builds/2705#019146bf-645a-4b43-b1db-597856afaf3b/403-614 but is holding up the CI for more than a day 😓

@isaacsas
Copy link
Member

Maybe @TorkelE knows how long it should take. It is a long one but I have no idea if it should be days.

@ChrisRackauckas
Copy link
Member Author

Well it's done now. Looks like it takes 60 hours.

@TorkelE
Copy link
Member

TorkelE commented Aug 15, 2024

60 hours doesn't seem too outlandish for the BCR one.

@isaacsas
Copy link
Member

Has the BCR sparse Jacobian compiliation gotten worse? 80 minutes now seems pretty bad and worse than I remember.

Also @ChrisRackauckas seems like there are Symbolics related errors in the WP plotting now?

@ChrisRackauckas
Copy link
Member Author

I don't think it's symbolic, but yes I need to investigate what happened there.

@ChrisRackauckas ChrisRackauckas force-pushed the biobump branch 2 times, most recently from 1997def to 2fc5137 Compare September 14, 2024 15:11
@isaacsas
Copy link
Member

isaacsas commented Nov 6, 2024

@ChrisRackauckas is this good to merge now? @vyudu has another PR to add a new benchmark but we should wait on that one till the updated Project.toml and Manifest.toml are merged.

@ChrisRackauckas
Copy link
Member Author

This still has a few failures IIUC from the CI

@ChrisRackauckas
Copy link
Member Author

Okay I'm going to merge and just fix Bikhori separately since that requires a lot less compute.

@ChrisRackauckas ChrisRackauckas merged commit c82f9f5 into master Nov 19, 2024
2 checks passed
@ChrisRackauckas ChrisRackauckas deleted the biobump branch November 19, 2024 17:48
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.

4 participants