Fix for user-defined inlet velocity profile failure by ktbolt · Pull Request #556 · SimVascular/svMultiPhysics

ktbolt · 2026-05-27T16:21:43Z

This is a simple fix for #555.

There is no CI test for a user-defined inlet velocity profile.

codecov · 2026-05-27T17:42:46Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.08%. Comparing base (5118d56) to head (3134a3e).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #556   +/-   ##
=======================================
  Coverage   69.08%   69.08%           
=======================================
  Files         181      181           
  Lines       34256    34257    +1     
  Branches     5931     5932    +1     
=======================================
+ Hits        23665    23666    +1     
  Misses      10454    10454           
  Partials      137      137

☔ View full report in Codecov by Harness.
📢 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.

hanzhao2020 · 2026-05-29T21:56:28Z

Looks good to me!

Copilot

Pull request overview

Fixes a solver initialization issue that breaks user-defined inlet velocity profiles (Issue #555) by avoiding an unconditional reallocation of bcType::gx during boundary-condition initialization.

Changes:

Preserve previously-read user-defined spatial profile data in lBc.gx by only allocating when it is empty.
Prevents user-defined profile values from being reset to zero during bc_ini().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

aabrown100-git · 2026-05-30T01:49:43Z

+  // lBc.gx may have values set when for example when 
+  // reading in a user-defined profile.
+  if (lBc.gx.size() == 0) {
+    lBc.gx.resize(lFa.nNo);
+  }


@ktbolt is it worth validating that lBc.gx.size() == lFa.nNo if it's not 0?

aabrown100-git · 2026-05-30T01:50:32Z

-  lBc.gx.resize(lFa.nNo);
-  //if (.NOT.ALLOCATED(lBc.gx)) ALLOCATE(lBc.gx(lFa.nNo))
+
+  // lBc.gx may have values set when for example when 


@ktbolt do you have a test you can add for user-defined spatial profile?

aabrown100-git · 2026-05-30T01:46:20Z

+
+  // lBc.gx may have values set when for example when 
+  // reading in a user-defined profile.
+  if (lBc.gx.size() == 0) {


Interesting, so previously any time a user defined a spatial profile the solver would fail?

@aabrown100-git Correct; there are no CI tests for the Spatial_profile_file_path parameter.

aabrown100-git · 2026-05-30T01:49:43Z

+  // lBc.gx may have values set when for example when 
+  // reading in a user-defined profile.
+  if (lBc.gx.size() == 0) {
+    lBc.gx.resize(lFa.nNo);
+  }


@ktbolt is it worth validating that lBc.gx.size() == lFa.nNo if it's not 0?

aabrown100-git · 2026-05-30T01:50:32Z

-  lBc.gx.resize(lFa.nNo);
-  //if (.NOT.ALLOCATED(lBc.gx)) ALLOCATE(lBc.gx(lFa.nNo))
+
+  // lBc.gx may have values set when for example when 


@ktbolt do you have a test you can add for user-defined spatial profile?

ktbolt · 2026-05-30T02:25:23Z

@aabrown100-git It might be good to check that lBc.gx.size() == lFa.nNo although in the current code lBc.gx is only resized for lFa.nNo.

ktbolt · 2026-06-01T17:13:52Z

@aabrown100-git I am undecided about adding a CI test since the user-defined profile impact such only a small section of code; don't want to have too many CI cases. Maybe this should be a Unit Test ?

aabrown100-git · 2026-06-01T21:55:36Z

@aabrown100-git I am undecided about adding a CI test since the user-defined profile impact such only a small section of code; don't want to have too many CI cases. Maybe this should be a Unit Test ?

@ktbolt I think a CI test would be good. We could do a unit test too, but would a unit test have caught this bug? I'm thinking a unit test would verify that user-defined profile data is read in correctly, but would we need another unit test to verify that it is not inadvertently overwritten like in this case?

A simple CI test could be just another variation of our fluid/pipe_RCR_... tests, like pipe_RCR_user_defined_profile

ktbolt · 2026-06-03T00:28:00Z

@aabrown100-git We would need two unit tests

Test reading velocity profile text file
Test that lBc.gx is computed correctly

My feeling is that this code is somewhat isolated from other code development so a CI test is not needed; also worried about the proliferation of CI tests.

aabrown100-git · 2026-06-03T17:13:05Z

@ktbolt Sounds good, doesn't hurt to add those unit tests. Would the second test check lBc.gx as it is passed through several functions, or are you thinking just testing bc_ini()?

Regarding the proliferation of CI tests, are you concerned about the testing time becoming too long? Perhaps we could start creating a different type of CI test that just checks the simulation runs without error, without checking that the solution matches some reference. We could make these very fast (1 timestep, 1 Newton iteration, big tolerances).

ktbolt · 2026-06-04T00:02:54Z

@aabrown100-git I was thinking to test baf_ini_ns::bc_ini since that is where lBc.gx is computed.

I am worried about the testing time as we keep adding more CI tests for added functionality (2D struct, electromechanics, etc.) and having tests that are really not integration tests.

CI tests checking to see if a simulation just runs without an error is an interesting idea but I think the cases it would be useful for could just be covered by unit tests maybe.

Note that we will start adding Example large and complex simulations for the User Guide ; some will need to be run on an HPC system. These simulations will be run and the results evaluated on a regular basis, maybe once a month or so or when a major code change is merged; would be good to collect execution times too.

aabrown100-git · 2026-06-04T05:12:48Z

@ktbolt Okay sounds good about the unit tests!

Add check if lBc.gx is already allocated.

533af1a

Merge branch 'main' into User-defied-inlet-velocity-profile-fails_555

75715f1

ktbolt requested review from aabrown100-git and hanzhao2020 May 29, 2026 21:20

hanzhao2020 approved these changes May 29, 2026

View reviewed changes

aabrown100-git requested a review from Copilot May 30, 2026 01:42

Copilot started reviewing on behalf of aabrown100-git May 30, 2026 01:42 View session

Copilot AI reviewed May 30, 2026

View reviewed changes

aabrown100-git requested changes May 30, 2026

View reviewed changes

Merge branch 'main' into User-defied-inlet-velocity-profile-fails_555

78a8227

aabrown100-git and others added 2 commits June 9, 2026 09:30

Merge branch 'main' into User-defied-inlet-velocity-profile-fails_555

789e722

Merge branch 'main' into User-defied-inlet-velocity-profile-fails_555

3134a3e

Conversation

ktbolt commented May 27, 2026

Uh oh!

codecov Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hanzhao2020 commented May 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

aabrown100-git May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabrown100-git May 30, 2026

Choose a reason for hiding this comment

Uh oh!

aabrown100-git May 30, 2026

Choose a reason for hiding this comment

Uh oh!

ktbolt May 30, 2026

Choose a reason for hiding this comment

Uh oh!

aabrown100-git May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabrown100-git May 30, 2026

Choose a reason for hiding this comment

Uh oh!

ktbolt commented May 30, 2026

Uh oh!

ktbolt commented Jun 1, 2026

Uh oh!

aabrown100-git commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ktbolt commented Jun 3, 2026

Uh oh!

aabrown100-git commented Jun 3, 2026

Uh oh!

ktbolt commented Jun 4, 2026

Uh oh!

aabrown100-git commented Jun 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented May 27, 2026 •

edited

Loading

aabrown100-git May 30, 2026 •

edited

Loading

aabrown100-git May 30, 2026 •

edited

Loading

aabrown100-git commented Jun 1, 2026 •

edited

Loading