[Bug] min_radial_dist unit mismatch in MotorUnitSim.calc_sfaps() — µm value used in mm context

[veylantis]

Description

In motor_unit_sim.py, the default min_radial_dist converts fiber diameter to micrometers, but the result is then compared against radial distances in millimeters:

# motor_unit_sim.py, line ~274:
min_radial_dist = float(
    np.mean(self.muscle_fiber_diameters__mm) * 1000
)  # Convert to micrometers

However, radial_distance (line ~345) is computed from muscle_fiber_centers__mm and electrode_positions, both in mm. The clamp comparison if radial_distance < min_radial_dist then mixes µm with mm.

For a typical muscle model with mean fiber diameter ~0.0525 mm:

• min_radial_dist = 0.0525 × 1000 = 52.5 mm
• Muscle radius ≈ 18 mm → max fiber-to-electrode distance ≈ 36 mm
• All fibers get clamped to r = 52.5 mm

Impact

• All fibers appear equidistant from the electrode in the XY plane
• Moving the electrode in X/Y has zero effect on the MUAP waveform for the same motor unit (xcorr = 1.000000)
• Only Z-axis displacement affects the output (Z is not clamped)
• MUAP PtP amplitudes are suppressed (~0.15 mV vs ~5.7 mV at the corrected scale)

Suggested Fix

Remove the * 1000 factor so min_radial_dist stays in mm (matching the coordinate system):

 if min_radial_dist is None:
     min_radial_dist = float(
-        np.mean(self.muscle_fiber_diameters__mm) * 1000
-    )  # Convert to micrometers
+        np.mean(self.muscle_fiber_diameters__mm)
+    )  # Mean fiber diameter in mm (prevents 1/r singularity)

Version

MyoGen v0.9.0 (current main)

[RaulSimpetru]

Hi @veylantis, thank you very much for opening these two issues. We are aware of both points and are currently planning a broader revamp of the EMG generation system, since the cylindrical model we currently use is quite simplified.

We are also planning to establish a MyoGen oversight committee to handle such issues in a more organized and transparent way going forward.

Would you be willing to open a PR integrating both suggestions into the code? I think it would be in everyone’s interest to address these issues now, since the larger revamp will likely take some time, especially as we will probably publish the new approach first.

Thanks again for pointing this out.

On a slightly unrelated note, I would also love to hear what you are planning to use MyoGen for. I saw that you work on radar simulations, so I am curious whether there is an interesting connection there or whether this is a separate project.

[veylantis]

Thank you for the quick response! I'll adapt the code to MyoGen's coding standards and open a separate PR for each:

1. The min_radial_dist unit fix
2. The GPU-accelerated SFAP backend for simulate_muaps()

We'll make sure to properly cite MyoGen and comply with the AGPL-3.0 license when we publish our work.

Regarding our use case: this is a separate project from the radar work. We use MyoGen to generate synthetic MUAP templates for a triplet loss model — harvesting waveforms at varying electrode displacements to build positive pairs for contrastive learning.

Looking forward to contributing!

[veylantis]

Hi! @RaulSimpetru
Opened PR #10 for the min_radial_dist fix.
The GPU backend PR will follow separately.

[veylantis]

The GPU backend PR is now ready: #11

Quick summary of results on RTX 3090:

• 8.2× speedup at default spatial resolution (dz=0.1, 200 fibers × 25 electrodes)
• 27.3× speedup at high resolution (dz=0.01, ~6000 spatial points per fiber)
• 3-way numerical parity verified (RMSE < 2e-19)

The implementation is model-agnostic — uses an xp=np backend parameter in the bioelectric functions, so any future model changes work automatically. CuPy is an optional dependency (pip install myogen[gpu]), CPU fallback is seamless.