Fixed lambda function overwriting issue

[JonathanPham]

Addresses Issue #44.

Each time we create a new lambda function for the boundary conditions in the create_outlet_bc_blocks() function in solver.py, the previous lambda function gets overwritten by the newest lambda function. Someone else on StackOverFlow has a similar issue here. To resolve this issue, I had to define the lambda function for the BCs in a separate function, create_unsteady_bc_value_function(). This is definitely a strange Python bug.

After fixing the issue above, I compared the 0d simulation results for all VMR models for 2 different cases: 1) using the old text input file and 2) using the new json format (with the BloodVessel block). The input files that I used are attached here: json_vs_text.zip. In this .zip file, there is a README.txt file that lists the source of these json and text files.

Note that in order to simulate the 0d models with the old text input file format, I had to use an older version of svZeroDSolver.

According to my tests, the 0d simulation results between the json and text cases match for all VMR models except for:

• 0002
• 0172
• 0173
• 0183
• 0184
• 0185
• 0186
• 0187
• 0189 Not sure why these models failed the tests, but maybe recent updates in SimVascular changed the input files a little bit.

[ktbolt]

@JonathanPham What is the reason for using lambda functions? They are not often used in Python except for sorting and such. And it is confusing to see statements like R_func = lambda x: R that don't use the bound variable x.

Rather than defining these guys locally as lambdas it would be better just to define functions that are documented with clear behavior.

[JonathanPham]

@ktbolt No reason for specifically using lambda functions. They were just easy to work with. Thanks for your feedback. I removed the lambda functions and instead, I defined a local function for the constant boundary conditions. Note that we still need to provide a "time" input to these functions, so that they can work with the general blocks, e.g. UnsteadyRCRBlockWithDistalPressure, in blocks.py.

With this new update, I am currently re-performing the aforementioned tests to confirm that the 0d simulation results for the VMR models agree between 1) the case using the new json input file and 2) the case using the old text input file. Will inform you if this test passes or not.

[mrp089]

Thanks for the quick fix @JonathanPham! I’m very surprised the test cases didn’t pick that up. Do you know why? Can we extend our testing to cover something like this in the future?

[JonathanPham]

@mrp089 I believe the test cases didn't pick up on this lambda function overwriting issue because they did not include models with multiple (different) outlet boundary conditions. There is a simple bifurcation test with multiple outlets, but these outlets used the same boundary condition (so there was only one lambda function for the outlet BC in the entire model). I'll write a new test to include this case.

[mrp089]

Perfect!

[JonathanPham]

Added a new test case: simple bifurcation model with different outlet BCs. Confirmed that this test fails when using this version of svZeroDSolver. Test passes using the updates in this pull request.

[JonathanPham]

I just finished the comparison of the 0d simulation results for all VMR models between 1) the new json input file and 2) the old text input file (using the same inputs given above). The results match for all models except for the same models mentioned yesterday:

• 0002
• 0172
• 0173
• 0183
• 0184
• 0185
• 0186
• 0187
• 0189

I think the updates for svZeroDSolver on this pull request are ready to merge, @mrp089.

[ktbolt]

@JonathanPham Very good! Btw, my comments are suggestions for future enhancements, no need to implement them immediately, don't want to hold up your progress.

[JonathanPham]

@ktbolt Thanks for the advice! I appreciate it. I'm slowly improving my "software engineering" skills haha.