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

Fix broken tests #1070

Merged
merged 4 commits into from
Oct 3, 2024
Merged

Fix broken tests #1070

merged 4 commits into from
Oct 3, 2024

Conversation

TorkelE
Copy link
Member

@TorkelE TorkelE commented Sep 27, 2024

Fixes the tests that have been broken by recent updates.

The long running for identifiability analysis has to do with us using array variables for representing unknown quantities, and there seems to be a problem with this (likely related to recent updates). Might be easiest to simple rewrite code to not use that, will have a look.

Comment on lines +65 to +70
0.1 + hill(X,5.0,K,3), 0 --> X
1.0, X --> 0
end
@unpack X, v, K, n, d = bistable_switch
@unpack X, K = bistable_switch
u0_guess = [X => 1.0]
p_start = [v => 5.0, K => 2.5, n => 3, d => 1.0]
p_start = [K => 2.5]
Copy link
Member

Choose a reason for hiding this comment

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

Do multiple parameters that are symbolic no longer work with the extension? Why do you need to change the parameters to hard-coded values?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, still work (see other tests).

The problem is that I wrote a test which evaluates some internal MTK functions and check that they return the same stuff. However, this kind of depended on ordering, and the test fails due to the parameter ordering not being right. I changed the this test so that there was only a single parameter (so that parameter ordering is not really a worry).

Probably the test should just be removed instead, but for now I just tried to make a minimal change.

@TorkelE
Copy link
Member Author

TorkelE commented Sep 27, 2024

There is still some weird stuff with SI. I have been looking at it for a while and untangled it a bit, but still not fully sure how to finally fix it. SI being broken is probably what holds up the docs as well.

@isaacsas
Copy link
Member

Sounds good. Feel free to merge this then when you want.

@isaacsas
Copy link
Member

isaacsas commented Oct 3, 2024

@TorkelE I'm going to merge this as I don't want to hold off much longer on getting PRs merged and making a release with the array fixes.

@isaacsas isaacsas merged commit 7abfe0f into master Oct 3, 2024
4 of 5 checks passed
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