Skip to content

Initialize Cramér-von Mises test#42

Open
RobbeBohy wants to merge 6 commits into
molmod:mainfrom
RobbeBohy:feature/cramervonmises-test
Open

Initialize Cramér-von Mises test#42
RobbeBohy wants to merge 6 commits into
molmod:mainfrom
RobbeBohy:feature/cramervonmises-test

Conversation

@RobbeBohy
Copy link
Copy Markdown
Collaborator

This PR introduces the first component of the new 3_validation workflow, as discussed in #27, and includes:

  • Initialization of the workflow.
  • A validation script that checks whether the sampled trajectories are consistent with the theoretical autocorrelation function (ACF), by comparing the distribution of $x(t + \Delta_t) - x(t)$ against the predicted distribution using the Cramér–von Mises test.

@RobbeBohy RobbeBohy requested a review from tovrstra May 20, 2026 15:20
Copy link
Copy Markdown
Member

@tovrstra tovrstra left a comment

Choose a reason for hiding this comment

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

Just some minor suggestions below and a few general ones here:

  • I would remove the _list suffixes of the variables names. The naming conventions I've used so far is somewhat fortranic, but works well in Python too: If a variable or an array represents a count of some sort, then prefix it with n (short for number of). If a variable is a list or array, add an s suffix (plural). Most people inuitively use an s suffix for both, which makes sense in a natural language, but does not work as a naming convention. Adding _list or _array instead of s is also common, but goes a bit against Python's duck typing idea.
  • I've tested the code checked the plots. It all looks clear. It may be helpful to add the "p-value of the p-values somewhere on or above the bottom panel. (There may be better names for this thing.)

Comment thread 3_validation/scripts/check_acf_consistency.py Outdated
unzipped_kernel = np.load(path_kernel)
acf = unzipped_kernel[acf_path]

pool = [[] for _ in range(len(dt_list))]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor suggestion: It would be more Pythonic to combine dts and pool into a single dictionary.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good idea, I refactored this in 902d772.

@RobbeBohy
Copy link
Copy Markdown
Collaborator Author

Thanks for the comments. I have added the p-value in a box on the bottom panel like this:
Scherm­afbeelding 2026-05-21 om 13 06 10

@RobbeBohy RobbeBohy requested a review from tovrstra May 21, 2026 11:07
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.

2 participants