Skip to content

Conversation

@kko27
Copy link
Contributor

@kko27 kko27 commented Aug 27, 2025

Adds a test case that reads in Fourier coefficients for boundary condition and fixes the read io bug.

Current situation

Resolves #418 (indexing issue when reading in Fourier coefficient files)

Release Notes

  • Added pipe_RCR_3d_fourier_coeff test case
  • Test case includes python script fft_temporal_values.py that can convert the .flow file into a .fcs (fourier coefficients) file
  • Added unit test (test_fft) that tests the fft function
  • Fixed the indexing issue when reading in .fcs files

Documentation

Code is documented with comments. Test case includes an updated README.

Testing

Test case is included as pipe_RCR_3d_fourier_coeff. Unit test has also been included.

Code of Conduct & Contributing Guidelines

@kko27 kko27 requested review from aabrown100-git and ktbolt August 27, 2025 19:22
@codecov
Copy link

codecov bot commented Aug 28, 2025

Codecov Report

❌ Patch coverage is 93.02326% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.07%. Comparing base (045220a) to head (0df4ddc).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
Code/Source/solver/read_files.cpp 50.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #427      +/-   ##
==========================================
+ Coverage   66.90%   67.07%   +0.17%     
==========================================
  Files         165      166       +1     
  Lines       33711    33746      +35     
  Branches     5672     5673       +1     
==========================================
+ Hits        22553    22636      +83     
+ Misses      11020    10972      -48     
  Partials      138      138              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@aabrown100-git aabrown100-git left a comment

Choose a reason for hiding this comment

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

Nice job with this! Just added a few suggestions and questions for clarification.

@ktbolt
Copy link
Collaborator

ktbolt commented Sep 3, 2025

@aabrown100-git Having a script in a test directory means that it does something else than just testing the solver.

The Python script is indeed useful so let's put it someplace users can find it ! I can add documentation for a utilities directory.

Copy link
Collaborator

@aabrown100-git aabrown100-git left a comment

Choose a reason for hiding this comment

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

Looks great, love the extensive documentation!

@kko27
Copy link
Contributor Author

kko27 commented Sep 11, 2025

@ktbolt, can we merge or did you want to review it one last time?

@ktbolt
Copy link
Collaborator

ktbolt commented Sep 11, 2025

Very nice documentation I agree !

@ktbolt ktbolt merged commit 6ae4572 into SimVascular:main Sep 11, 2025
6 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.

Undefined behavior when reading in Fourier coefficients files - indexing issue

3 participants