Jump diffusion model #95

henrikjacobsenfys · 2026-02-04T21:49:01Z

Add Jump Diffusion and remove some unit options that were bugged in the Brownian diffusion. Also had Claude AI fix the weakref bug that had been driving me crazy. It's a temporary solution, but anything is better than getting these random errors that are not related to the code I've written.

Closes #78
closes #66 as irrelevant

codecov · 2026-02-04T22:05:25Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (develop@568a59a). Learn more about missing BASE report.

Additional details and impacted files

@@            Coverage Diff             @@
##             develop      #95   +/-   ##
==========================================
  Coverage           ?   97.75%           
==========================================
  Files              ?       31           
  Lines              ?     1689           
  Branches           ?      274           
==========================================
  Hits               ?     1651           
  Misses             ?       21           
  Partials           ?       17

Flag	Coverage Δ
integration	`0.00% <0.00%> (?)`
unittests	`97.75% <100.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

rozyczko

Good code. Only typos and minor issues found.

rozyczko · 2026-02-05T07:59:44Z

src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py

@@ -1,17 +1,11 @@
-# SPDX-FileCopyrightText: 2025-2026 EasyDynamics contributors <https://github.com/easyscience>
-# SPDX-License-Identifier: BSD-3-Clause
-


Any particular reason you deleted the license header?

Copy/paste from old code going wrong

rozyczko · 2026-02-05T08:00:19Z

src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py

+    def create_component_collections(
+        self,
+        Q: Q_type,
+        component_display_name: str = 'Brownian translational diffusion',


This should probably say "Jump translational diffusion"

See above haha

rozyczko · 2026-02-05T08:00:50Z

src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py

+        """Create ComponentCollection components for the Brownian.
+
+        translational diffusion model at given Q values. Args:
+        ---------- Q : Number, list, or np.ndarray
+            Scattering vector values.
+        component_display_name : str
+            Name of the Brownian Diffusion Lorentzian component.
+        Returns
+        -------
+        List[ComponentCollection]
+            List of ComponentCollections with Brownian Diffusion
+            Lorentzian components.
+        """


same here s/Brownian/translational/g

rozyczko · 2026-02-05T08:01:30Z

src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py

+        unit_conversion_factor_nominator = (
+            self._hbar * self.diffusion_coefficient / (self._angstrom**2)


nominator -> did you mean numerator?

rozyczko · 2026-02-05T08:02:33Z

tests/unit/easydynamics/sample_model/diffusion_model/test_jump_translational_diffusion.py

+angstrom = DescriptorNumber('angstrom', 1e-10, unit='m')
+
+
+class TestJumpTranslationalDiffusion:


Missing tests for test_scale_setter and test_scale_setter_raises (since they are in the Brownian tests)

I moved the scal setter and getter to diffusion_base, now the tests are also there

rozyczko · 2026-02-05T08:28:06Z

src/easydynamics/sample_model/diffusion_model/diffusion_model_base.py

        except Exception as e:
            raise UnitError(
-                f'Invalid unit: {unit}. Unit must be a string or scipp Unit and convertible to meV.'  # noqa: E501
+                f'Invalid unit: {unit}. Unit must be a string or scipp Unitand convertible to meV.'


typo: Unitand -> Unit and

rozyczko · 2026-02-05T08:32:22Z

src/easydynamics/sample_model/diffusion_model/jump_translational_diffusion.py

+            raise TypeError('Q must be a float.')
+
+        # Q is given as a float, so we need to add the units
+        return f'hbar * D* {Q} **2*1/(angstrom**2)/(1 + (D * t* {Q} **2/(angstrom**2)))'


Why the extra 1 in the expression?
Just {Q}**2 / (angstrom**2) should be fine

I think I had problems with it related to the bug with order of operations when dividing parameters

seventil

lgtm, just nitpicks

seventil · 2026-02-05T13:46:37Z

src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py

@@ -62,65 +57,35 @@ def __init__(
            Defaults to "meV".
        scale : float or Parameter, optional


typo here, scale is numeric

henrikjacobsenfys · 2026-02-05T14:08:42Z

src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py

+    # ------------------------------------------------------------------
+    # dunder methods
+    # ------------------------------------------------------------------
+


Same for diffusion_coefficient and all other repr I've made so far... I'll streamline them once I know what I want, which is after the Job class :) but it's a good point!

henrikjacobsenfys · 2026-02-05T14:08:42Z

src/easydynamics/sample_model/diffusion_model/brownian_translational_diffusion.py

+    # ------------------------------------------------------------------
+    # dunder methods
+    # ------------------------------------------------------------------
+


Same for diffusion_coefficient and all other repr I've made so far... I'll streamline them once I know what I want, which is after the Job class :) but it's a good point!

* jump diffusion model * fix notebook * fix weakref? * fix weakref? * fix weakref??? * ... * weakref... * claude fixed it? * respond to PR comments * Move scale test to base model and add some formatting * fix typo * small fix

henrikjacobsenfys added 8 commits February 4, 2026 20:22

jump diffusion model

32acbe8

fix notebook

d2c80c0

fix weakref?

6749a8b

fix weakref?

8d6906d

fix weakref???

7211f38

...

2c32af9

weakref...

01a5d6b

claude fixed it?

9012056

henrikjacobsenfys added [scope] enhancement Adds/improves features (major.MINOR.patch) [priority] medium Normal/default priority labels Feb 4, 2026

henrikjacobsenfys marked this pull request as ready for review February 4, 2026 21:49

henrikjacobsenfys mentioned this pull request Feb 5, 2026

Add Jump Translational DIffusion model #80

Closed

henrikjacobsenfys requested a review from damskii9992 February 5, 2026 06:03

rozyczko approved these changes Feb 5, 2026

View reviewed changes

henrikjacobsenfys added 4 commits February 5, 2026 10:28

respond to PR comments

6367366

Move scale test to base model and add some formatting

e071164

fix typo

a87c053

small fix

a1901ff

seventil reviewed Feb 5, 2026

View reviewed changes

henrikjacobsenfys commented Feb 5, 2026

View reviewed changes

henrikjacobsenfys merged commit 88af3d9 into develop Feb 5, 2026
34 checks passed

		@@ -1,17 +1,11 @@
		# SPDX-FileCopyrightText: 2025-2026 EasyDynamics contributors <https://github.com/easyscience>
		# SPDX-License-Identifier: BSD-3-Clause

		unit_conversion_factor_nominator = (
		self._hbar * self.diffusion_coefficient / (self._angstrom**2)

		angstrom = DescriptorNumber('angstrom', 1e-10, unit='m')


		class TestJumpTranslationalDiffusion:

		@@ -62,65 +57,35 @@ def __init__(
		Defaults to "meV".
		scale : float or Parameter, optional

Jump diffusion model #95

Jump diffusion model #95

Uh oh!

Conversation

henrikjacobsenfys commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rozyczko left a comment

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!

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!

seventil left a comment

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

henrikjacobsenfys commented Feb 4, 2026 •

edited

Loading

codecov bot commented Feb 4, 2026 •

edited

Loading