adding fiber generation codes #480

javijv4 · 2025-12-05T19:42:27Z

Current situation

Resolves #479

Release Notes

Added fiber generation codes in the utilities folder.

Code of Conduct & Contributing Guidelines

I agree to follow the Code of Conduct and Contributing Guidelines.

aabrown100-git

@javijv4 great job thanks for doing this! Sorry for the delay in reviewing, but I left some comments throughout.

My only major comment is to better explain the modifications to Bayer, and relatedly if we can validate the implementations for both Bayer and Doste by comparing to lifex or other codes. I think both of these aspects will be useful to future lab members, especially when writing about these methods in their papers.

utilities/fiber_generation/src/templates/solver_bayer.xml

utilities/fiber_generation/src/templates/solver_doste.xml

utilities/fiber_generation/README.md

aabrown100-git · 2025-12-22T19:11:55Z

utilities/fiber_generation/README.md

+### Updates to the old code
+* All operations are vectorized now.
+* The SVmultiphysics solver now solves a Laplace equation.
+* In Bayer: For the bislerp interpolation, instead of using the correction described in Bayer et al. (that returns a discontinuity), the basis are flipped to maintain a coherent fiber direction (see function `generate_fibers_BiV_Bayer_cells` in `FibGen.py`).


Can you explain these modifications a bit more? They're not published anywhere right? If so, I think it would be good to explain these in more detail (with math perhaps) and explain why you made this modifications.

Hmm.. do you refer to explaining all three of these points?

Explaining the vectorization and the Bayer modification with math would probably require rewriting the methods of the Bayer paper again, which might be a bit overkill for a README? Maybe I can add those details to the documentation?

For the Bayer bislerp one, I've been trying to look for a math or code explanation that explains why the formula returns this discontinuity, but I haven't found it. If I rewrite the methods of the Bayer paper in the documentation, then I can indicate exactly where I performed the flip of the vectors. Would that work?

Same for the Laplace, I think including math would be better suited for the documentation.

Let me know what you think

Sounds good with the documentation! I don't think you need to explain the vectorization or the Laplace equation part. Just explain how this implementation is different from what's published in the Bayer paper

utilities/fiber_generation/example/ot/doste.png

aabrown100-git · 2025-12-22T19:15:39Z

utilities/fiber_generation/README.md

@@ -0,0 +1,35 @@
+# SV-fibergen
+Python + SVmultiphysics codes for fiber generation. Two methods are implemented:
+* Bayer et al. (2012). [link](https://doi.org/10.1007/s10439-012-0593-5)


Is there any validation we can do? I know you were looking into lifex, were you able to get that to work with your example data? It would be great to show that this code produces the same fiber field as lifex

@aabrown100-git lifex-fibers does not (as far as I could find) directly support biventricular fiber generation. My guess is that they use the code to get LV/RV fibers and then use some kind of interpolation (probably bislerp) to get the BV fibers.

The way I usually check the code is to, after generating the fibers, calculate the angle between the fiber direction and the circumferential vector. These should match the alpha/beta values. The other check is to check that the alpha/beta angle is changing linearly with the transmural coordinate. Will it be okay to add a VALIDATION.md showing these checks?

Sure a validation document sounds good!

utilities/fiber_generation/README.md

aabrown100-git · 2026-01-07T21:08:34Z

utilities/fiber_generation/src/templates/solver_bayer.xml

+	</Output>
+
+	<Output type="Alias" >
+		<Temperature> Phi_BiV_D</Temperature>


How is Phi_BiV_D used?

Good catch! It's not doing anything. That problem was in the original file I got to generate the Bayer fibers, so I am guessing it is an auxiliary problem for something else, not the fibers. I deleted it.

aabrown100-git · 2026-01-07T21:15:00Z

utilities/fiber_generation/src/FibGen.py

+    gPhi_EPI, gPhi_LV, gPhi_RV, gPhi_AB = loadLaplaceSolnBayer(laplace_results_file)
+
+
+    # Write the fiber directions to a vtu files


Is this comment correct?

nope, I fixed it

aabrown100-git · 2026-01-07T21:15:39Z

utilities/fiber_generation/src/FibGen.py

+    print('\n   Total time: %.3fs' % (t2-t1))
+    print("========================================================")
+
+    if return_angles:


Should this go before the vtu file is written?

aabrown100-git · 2026-01-07T21:48:46Z

utilities/fiber_generation/README.md

+* All operations are vectorized now.
+* The SVmultiphysics solver now solves a Laplace equation.
+* In Bayer: For the bislerp interpolation, instead of using the correction described in Bayer et al. (that returns a discontinuity), the basis are flipped to maintain a coherent fiber direction (see function `generate_fibers_BiV_Bayer_cells` in `FibGen.py`).
+* In Bayer: The beta angles were not being included correctly. The second rotation was being applied respect the first vector (circumferential) when it should be respect to the second vector (longitudinal) (see function `generate_fibers_BiV_Bayer_cells` in `FibGen.py`).


Can you also justify this too?

Same as before, should I add it to the documentation?

Yes add to documentation

dseyler · 2026-01-07T22:07:51Z

As discussed with @javijv4, I've found that computing gradients at nodes (where Laplace solutions are stored) and then interpolating point data to cell data gives smoother basis vectors than the current method of first interpolating Laplace solutions to cells then computing gradients. I've also noticed that for some course meshes, 500 LS iterations doesn't quite converge (increased to max 2000 in my branch). See https://github.com/dseyler/sv-fibergen.git where these changes have been implemented as well as optional command line inputs.

javijv4 · 2026-01-07T22:38:00Z

@dseyler thank you! It looks great.

@aabrown100-git I will

Merge @dseyler's updates
Add validation documentation/test code that, given a fiber field and an orthogonal basis, calculates the alpha and beta angles and returns the error between the prescribed and output values
Write down the equations in the documentation

Does that sound good?

aabrown100-git · 2026-01-07T22:45:49Z

@javijv4 Sounds good to all points. For point 3, for the documentation is that the VALIDATION.md or another document?

javijv4 · 2026-01-07T22:56:35Z

@aabrown100-git For the VALIDATION.MD, I was thinking of restricting it to show that the code is doing what it is supposed to.
And for documentation, I was thinking the svMultiPhysics documentation, but maybe it is better to just have a documentation in the fiber-generation folder? I can also put the validation info in the documentation to avoid multiple .md besides the readme. What do you think?

aabrown100-git · 2026-01-07T23:06:35Z

@javijv4 VALIDATION.md sounds good. For the documentation, I think it's better to locate it in the fiber-generation folder (although others may disagree). Could you just make a DOCUMENTATION.md? Or maybe just put it in the existing README.md?

ktbolt · 2026-01-08T00:20:55Z

@javijv4 Sorry for the late review !

This Python code probably does not belong in the svMultiPhysics repository but should be part of SimVascular like the Purkinje fiber generation code is.

ktbolt · 2026-01-08T00:39:29Z

utilities/fiber_generation/main_bayer.py

+Created on 2025/11/21 20:38:14
+
+@author: Javiera Jilberto Vallejos 
+'''


@javijv4 There is no need have date and author. This will also need a license header.

@ktbolt I removed the date and author. Where can I find the license header?

ktbolt · 2026-01-08T00:40:49Z

utilities/fiber_generation/main_doste.py

+Created on 2025/11/21 20:38:14
+
+@author: Javiera Jilberto Vallejos 
+'''


@javijv4 No need for date and author, also add license header.

ktbolt · 2026-01-08T00:45:14Z

utilities/fiber_generation/src/FibGen.py

+        x: array-like of shape (N, 3)
+    Returns:
+        np.ndarray of shape (N, 3) with row-wise normalized vectors.
+    """


@javijv4 Be sure to follow a consistent docstring format like Google.

javijv4 · 2026-01-08T18:34:44Z

@ktbolt thanks! I will work on the revisions.

Regarding where to host the code, from what I found, I think the Purkinje code in the SimVascular repository is a SimVascular plugin. These, for now at least, are stand-alone Python codes. I discussed where to put this code with @aabrown100-git, and the utilities folder in svMultiPhysics seemed like a good place, since it already has two other folders that also contain stand-alone Python code. I can definitely move it somewhere else if you think there are better places to put it, though!

ktbolt · 2026-01-08T20:19:07Z

@javijv4 The core of the Purkinje Plugin is its Python code, it can be imported as a package and run in a Python script, the plugin just provides an interface to it. This functionality will be made available in the new SimVascular implementation in Slicer 3D as a plugin and Python package.

It seems that this fiber generation code should be made available to SimVascular users as a Python package that can be imported, something that could later be used to create a Slicer 3D plugin.

This will require a bit more work to generalize the code but it is better to do this now than later.

javijv4 · 2026-01-08T21:43:02Z

@ktbolt sounds good! How should I proceed with this then? The Purkinje plugin repository is an individual repo, so I am not sure how to go about creating one. Also, this pull request will need to be closed, right?

I have a separate GitHub repository (https://github.com/javijv4/sv-fibergen) where I initially had all these codes, and where I've been working on the revisions, so it would be easy to just transfer that to a SimVascular repository.

ktbolt · 2026-01-08T23:14:08Z

@javijv4 I think it will be ok to just leave the code where it is and we can figure out where to put it in future, not sure how the new Slicer SimVascular code will be organized.

The important thing is to implement a general Python package that can be imported, does not assume a solver or certain face names.

@aabrown100-git What do you think ?

aabrown100-git · 2026-01-09T05:50:32Z

@ktbolt @javijv4 I think implementing it as a Python package is a good idea! From what I can tell, FibGen.py is basically a Python package already. Do we just need a few changes to make it more user friendly?

@javijv4 If we want to remove the template .xml files, you can write Python code to generate the desired xml file and include this in FibGen.py. A few prompts to the AI should get it done.

@ktbolt what do you mean the package should not assume a solver?

ktbolt · 2026-01-09T17:55:04Z

@javijv4 @aabrown100-git I think FibGen.py needs to be more general

use generic face names for heart surfaces, I see if face_name == "epi": in the code
have an option for just reading a vtu results file or running the solver, not sure that the code should even run the solver, maybe just create the solver.xml file
encapsulate bayer and doste in classes

I had thought that the code could work on the output of any solver but maybe that is not important.

javijv4 · 2026-01-10T00:19:34Z

@ktbolt, sounds good, I can refactor the code to make it more general and use classes. This is the structure I am thinking of,

I think it makes sense for the code to be able to read the solutions from any other solver, which is possible with this structure. But I would like to keep the option to run the solver because, in practice, when running these codes for several patients, it is so much easier to have all the steps in one file.

About the first point, what do you mean by generic face names? The way I have it set up is that the user indicates the name of the surfaces in a dictionary in the main Python file,

surface_names = {'epi': 'EPI.vtp',
                 'epi_apex': 'EPI_APEX.vtp',    # New surface
                 'base': 'BASE.vtp',
                 'endo_lv': 'LV.vtp',
                 'endo_rv': 'RV.vtp'}

which is passed to the function that runs the Laplace problem that uses tit to point to the right files in the solver_*.xml files. I was trying to maintain the original code as much as possible, but since I will be adding classes, I can revise the input reading to be more straightforward as well.

@aabrown100-git sounds good, I will do that.

ktbolt · 2026-01-10T02:05:01Z

@javijv4 In def runLaplaceSolver I see that strings like "epi" are being used to check face names

        # Look ahead for Face_file_path
        if face_name and i + 1 < len(lines) and "<Face_file_path>" in lines[i + 1]:
            # Determine which file to use based on face name
            if face_name == "epi":
                new_path = os.path.join(surfaces_dir, surface_names['epi'])

Maybe "epi" is being used to identify a surface of the heart ? If that is true then it is better to use an Enum class. And use a descriptive name, not sure what "tv", "mv", etc. stand for.

javijv4 · 2026-01-12T21:31:46Z

Yeah, the strings are used to identify a particular heart surface. "epi" = epicardium, "mv" = mitral valve, "tv" = tricuspid valve. I will use more descriptive names and look into using an Enum class. Thanks!

javijv4 added 2 commits December 5, 2025 11:29

adding fiber generation codes

b010f79

increasing linear solver max iterations

c36a6b8

javijv4 requested review from aabrown100-git and ktbolt December 16, 2025 22:17

aabrown100-git requested changes Dec 22, 2025

View reviewed changes

javijv4 added 3 commits December 29, 2025 09:40

changing paths and README.md

c401a9a

adding .pngs for fiber, sheet, sheetnormal directions

f15a136

updating README.md

6fd7d15

aabrown100-git reviewed Jan 6, 2026

View reviewed changes

utilities/fiber_generation/README.md Show resolved Hide resolved

aabrown100-git reviewed Jan 7, 2026

View reviewed changes

ktbolt reviewed Jan 8, 2026

View reviewed changes

Merge branch 'main' into dev_#479

6941248

		gPhi_EPI, gPhi_LV, gPhi_RV, gPhi_AB = loadLaplaceSolnBayer(laplace_results_file)


		# Write the fiber directions to a vtu files

adding fiber generation codes #480

Are you sure you want to change the base?

adding fiber generation codes #480

Uh oh!

Conversation

javijv4 commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current situation

Release Notes

Code of Conduct & Contributing Guidelines

Uh oh!

aabrown100-git left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dseyler commented Jan 7, 2026

Uh oh!

javijv4 commented Jan 7, 2026

Uh oh!

aabrown100-git commented Jan 7, 2026

Uh oh!

javijv4 commented Jan 7, 2026

Uh oh!

aabrown100-git commented Jan 7, 2026

Uh oh!

ktbolt commented Jan 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javijv4 commented Jan 8, 2026

Uh oh!

ktbolt commented Jan 8, 2026

Uh oh!

javijv4 commented Jan 8, 2026

Uh oh!

ktbolt commented Jan 8, 2026

Uh oh!

javijv4 commented Dec 5, 2025 •

edited

Loading