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
Force the returned solution object to stay as ODESolution #6
Conversation
sol = _threadedsensitivities(react) | ||
return sol | ||
|
||
ReactionMechanismSimulator.threadedsensitivities = pythreadedsensitivities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does pythreadedsensitivities get defined anywhere? Also is it possible to move this up with pygetfluxdiagram?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this, the function was supposed to be pythreadedsensitivities
. I also move this up with pygetfluxdiagram
Do we not have tests for pyrms? |
There is a test, but it ran off travis so I think it might not work now. |
pyrms/rms.py
Outdated
# Allow us the get the solution object in the julia wrapped object without | ||
# it being converted into list of lists | ||
get = Main.pyfunctionret(Main.get, Main.Any, Main.PyAny, Main.PyAny, Main.PyAny) | ||
def pythreadedsensitivities(react, inds=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can these be fed the keyword arguments the original function can?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs ReactionMechanismGenerator/ReactionMechanismSimulator.jl#212 to be merged first
Passing in the keyword arguments as Dict{Symbol,Float64} is tricky. I had to change something in RMS to get the type right, see ReactionMechanismGenerator/ReactionMechanismSimulator.jl#212. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Can you squash as appropriate?
@mjohnson541 All squashed! |
ReactionMechanismSimulator.threadedsensitivities
is supposed to return either an ODESolution object, or a dictionary containing ODESolution as values, but they both get converted to list of lists when we use it through pyrms.Fixing the case that returns an ODESolution object is easier, I use the solution mentioned in JuliaPy/PyCall.jl#460.
Fixing the case that returns a dictionary of ODESolution objects is trickier. I had to force the
Main.get
to not convert the value into list of lists.