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.

Sign up for GitHub

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

Jump to bottom

Function input feature for time-dependent boundary conditions #108

Open
wants to merge 45 commits into
base: master
Choose a base branch
from
Open

Function input feature for time-dependent boundary conditions #108

wants to merge 45 commits into from

Conversation

kharold23
Copy link
Contributor

Current situation

Closes #89

Release Notes

  • Added a new parameter fn to flow and pressure boundary conditions that takes in a string
  • Interprets the string as a function of time, represented by the variable t
  • The value of the boundary condition is calculated at each time step by evaluating the function at t = the current time value.

Documentation

Time-dependent boundary conditions can now be input using a function passed in as a string. The function is of the variable t, or time. For example:
"bc_values": { "fn": "2.0 * (4*atan(1.)) * COS(2.0 * (4*atan(1.)) * t)" }

Testing

Test case test_time_dependent_block added to test_io.py, which runs json files timeDep_Flow.json and timeDep_Pressure.json.

Code of Conduct & Contributing Guidelines

@kharold23
expression_string (SEGFAULT)
4df6072 
Initial implementation of a time-dependent flow BC from an expression string. Currently results in a SEGFAULT.
@kharold23
Time dependent flow (fixed)
7fad1f0 
Finished debugging time-dependent flow block
@kharold23
Time-dependent pressure
fe12497
@kharold23
Add test cases and clean/format code
6460bf8
@kharold23
Documentation
8258e4e
@kharold23
Clang Format Fix?
ad3b585
@kharold23
Clang-format fix?
7acb97c
@kharold23
Update Parameter.cpp
a3411ba
@ktbolt
Copy link
Contributor

ktbolt commented May 29, 2024

I ran the pulsatileFlow_CStenosis_steadyPressure case on Ubuntu 22 and got the following exception

terminate called after throwing an instance of 'std::runtime_error'
  what():  Maximum number of non-linear iterations reached.

You need to add catching exceptions in the Python testing code.

ktbolt
ktbolt reviewed May 30, 2024
View reviewed changes
src/model/Parameter.cpp Outdated Show resolved Hide resolved
@ktbolt
Copy link
Contributor

ktbolt commented May 30, 2024

We don't want to mix external software with our source code so the exprtk.hpp file should not be in svZeroDSolver/src/. We need to add a ThirdParty directory and put the exprtk.hpp file in a sub-directory of it.

@ktbolt
Copy link
Contributor

ktbolt commented May 30, 2024

Also I see that there is Docker container file in svZeroDSolver/container/profiling for Ubuntu 22, you can use this to test I think.

@codecov Codecov
Copy link

codecov bot commented Jun 18, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 74.69880% with 21 lines in your changes missing coverage. Please review.

Project coverage is 25.12%. Comparing base (688cece) to head (e6760c6).

Files Patch % Lines
src/solve/SimulationParameters.cpp 73.80% 11 Missing ⚠️
src/model/Parameter.cpp 65.51% 10 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           master     #108       +/-   ##
===========================================
- Coverage   79.25%   25.12%   -54.14%     
===========================================
  Files          52       53        +1     
  Lines        2179    15401    +13222     
  Branches      278     1405     +1127     
===========================================
+ Hits         1727     3869     +2142     
- Misses        452    11532    +11080

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

menon-karthik
menon-karthik requested changes Jun 24, 2024
View reviewed changes
Copy link
Member

@menon-karthik menon-karthik left a comment

Choose a reason for hiding this comment

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

Almost ready! Just some minor comments.

src/model/FlowReferenceBC.cpp Outdated Show resolved Hide resolved
tests/cases/timeDep_Flow.json Outdated Show resolved Hide resolved
tests/cases/timeDep_Pressure.json Outdated Show resolved Hide resolved
src/model/Parameter.cpp Outdated Show resolved Hide resolved
tests/test_io.py Outdated Show resolved Hide resolved
@menon-karthik menon-karthik added the enhancement New feature or request label Jun 24, 2024
@menon-karthik menon-karthik mentioned this pull request Jun 27, 2024
Merged
@kharold23
Requested changes
8a4c875
@kharold23
Add results of the time dependent test cases
927d5cd
@kharold23
expression_string (SEGFAULT)
2741f35 
Initial implementation of a time-dependent flow BC from an expression string. Currently results in a SEGFAULT.
@kharold23
Time dependent flow (fixed)
772548b 
Finished debugging time-dependent flow block
@kharold23
Time-dependent pressure
10e293f
@kharold23
Add test cases and clean/format code
25070a1
@kharold23 kharold23 force-pushed the time-dependent branch 7 times, most recently from 640f3e7 to 563ac27 Compare July 31, 2024 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ktbolt ktbolt ktbolt left review comments

@menon-karthik menon-karthik menon-karthik requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow flow and pressure blocks to take time-dependent functions
3 participants
@kharold23 @ktbolt @menon-karthik