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

OpenCOR crashes due to a wrong assignment using Python #2526

Closed
WeiweiAi opened this issue Aug 10, 2021 · 8 comments · Fixed by #2529
Closed

OpenCOR crashes due to a wrong assignment using Python #2526

WeiweiAi opened this issue Aug 10, 2021 · 8 comments · Fixed by #2529
Assignees
Labels
Milestone

Comments

@WeiweiAi
Copy link

WeiweiAi commented Aug 10, 2021

Basic info: Window 10, OpenCOR Snapshot 2021-05-19

Description:
The crash is caused by a mistake of assignment using Python. It has nothing to do with any particular model.
Here is an example of a Python script:

import opencor as oc

# Load the simulation file
simfile='anyfile.sedml'
simulation = oc.open_simulation(simfile)

# The data object houses all the relevant information
# and pointers to the OpenCOR internal data representations
data = simulation.data()
# Modify the initial value of some state variabls and run the simulation
V_initial = [0]
data.states()['outputs/V'] = V_initial  ##[Here is a mistake]
simulation.run()
# Access simulation results
results = simulation.results()
...

The OpenCOR crashes whenever running the above Python script.
There is a mistake in the example, and it is supposed to be

data.states()['outputs/V'] = V_initial [0]

or

V_initial = 0
data.states()['outputs/V'] = V_initial

After the correction, the simulation works fine.
It would be good if the OpenCOR throws an error instead of crashing when encountering the error like in the example.
Thanks.

@agarny
Copy link
Contributor

agarny commented Aug 10, 2021

Thanks for the report @WeiweiAi. It would be nice if you could provide me with whatever files are known to reproduce the problem. It will not only save me time while working on your issue, but also if I ever need to come back to this issue.

@WeiweiAi
Copy link
Author

WeiweiAi commented Aug 10, 2021 via email

@agarny
Copy link
Contributor

agarny commented Aug 10, 2021

This is not working for me. Your workspace has been cloned to /Users/Alan/Desktop/64f. So, from the OpenCOR's Python Console window, I did:

cd /Users/Alan/Desktop/64f/sed-ml
run originalFig12_sim.py

which results in:

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
~/Desktop/64f/sed-ml/originalFig12_sim.py in <module>
     15 # Load the simulation file
     16 simfile='periodic-stimulus.sedml'
---> 17 simulation = oc.open_simulation(simfile)
     18 
     19 # The data object houses all the relevant information

ValueError: Error: the imports could not be fully instantiated (&lt;strong&gt;/Users/Alan/Desktop/64f/experiments/&lt;/strong&gt; imports &lt;strong&gt;../cellLib/Components/units.cellml&lt;/strong&gt;, which contents could not be retrieved).

From there, I tried to run the script from the command line (the preferred option for me):

cd /Users/Alan/Desktop/64f/sed-ml
[PathToOpenCOR]/pythonshell originalFig12_sim.py

and I am getting a crash... but I am getting a crash with your both your original script and the modified one.

So, at this stage, I can't reproduce your problem.

@agarny
Copy link
Contributor

agarny commented Aug 10, 2021

FTR, the problem was that the workspace contains a Git submodule which wasn't initialised. Once initialised, I can reproduce the problem as described by @WeiweiAi. Now, the question is whether we can catch a Python "error" and thus prevent OpenCOR from crashing...

@agarny agarny self-assigned this Aug 10, 2021
@agarny agarny added the Python label Aug 10, 2021
@agarny agarny added this to the 0.7 milestone Aug 10, 2021
@dbrnz
Copy link
Contributor

dbrnz commented Aug 10, 2021

Now, the question is whether we can catch a Python "error" and thus prevent OpenCOR from crashing...

AFAIK we do check for Python errors and respond appropriately...

I'm guessing that the underlying fault will be in a wrapper around one of our OpenCOR classes. In particular I understand the crash happens with:

V_initial = [0]
data.states()['outputs/V'] = V_initial

which is when a list is assigned into the states array instead of an expected number. @agarny, I'm happy to take a look if you would like me to.

agarny added a commit to agarny/opencor that referenced this issue Aug 10, 2021
@agarny
Copy link
Contributor

agarny commented Aug 10, 2021

Yes, I know exactly where the problem occurs: here. Looking at the code, the obvious fix seems to be this one, which now results in OpenCOR behaving better:
Screen Shot 2021-08-10 at 22 40 50
No idea why the value returned by PyNumber_Check() wasn't checked (my fault when I merged your code?). Either way, this now seems fine. Otherwise, I checked whether PyNumber_Check() is used in other places and it's not. So, I guess we can consider this issue fixed?

@dbrnz
Copy link
Contributor

dbrnz commented Aug 10, 2021

Looks good to close, thanks!

@agarny
Copy link
Contributor

agarny commented Aug 10, 2021

Great, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants