Skip to content

Fix GBM demo replication statistics and speed up the Gaussian transform - #585

Open
larissensium wants to merge 339 commits into
developfrom
geometric_brownian_motion
Open

Fix GBM demo replication statistics and speed up the Gaussian transform#585
larissensium wants to merge 339 commits into
developfrom
geometric_brownian_motion

Conversation

@larissensium

Copy link
Copy Markdown
Collaborator

Fixed a table in gbm_demo to correctly show results averaged over replications.

sou-cheng-choi and others added 30 commits December 7, 2025 09:14
README.md:
* Corrected filename references.
* Removed references to non-existent files.

averaged_mae.py:
* Fixed PEP 8 spacing for parameter qp_seed and use the parameter in code.
*Enhanced documentation with.
*Removed an unused import.
* Cleaned up code by removing a commented-out block.
@larissensium

Copy link
Copy Markdown
Collaborator Author

@sou-cheng-choi added some explanation and formulas to the demo

@sou-cheng-choi

sou-cheng-choi commented Aug 11, 2026

Copy link
Copy Markdown
Member

@larissensium

The symbol $\mu$ need to be disambiguated from the drift coefficient. Similar concern exists for $\sigma$.

@sou-cheng-choi

sou-cheng-choi commented Aug 12, 2026

Copy link
Copy Markdown
Member

@larissensium

  1. Pull recent changes from develop into current branch.

  2. generate_qmcpy_paths()in demos/GBM/gbm_code/qmcpy_util.py consistently returns a shape of (replications, n_paths, n_steps) even when replications=1.

  • Please fix the documentation.
  • The file data_util.py uses the indexing qp_final = qp_paths[:, -1]. It should use qp_final = qp_paths[..., -1] or qp_final = qp_paths[:,:, -1] to extract the terminal values.
from demos.GBM.gbm_code.qmcpy_util import generate_qmcpy_paths
S0, MU, SIGMA, T = 100.0, 0.05, 0.2, 1.0
paths, _ = generate_qmcpy_paths(S0, MU, SIGMA**2, T, n_steps=4, n_paths=8, replications=1)
wrong = paths[:, -1] 
terminal = paths[..., -1]
  1. generate_quantlib_paths are generating the same paths no matter how seed value changes. For example,
    from demos.GBM.gbm_code.quantlib_util import generate_quantlib_paths
    kwargs = dict(initial_value=S0, mu=MU, sigma=SIGMA, maturity=T, n_steps=3, n_paths=2)
    paths = {seed: generate_quantlib_paths(**kwargs, sampler_type="Sobol", seed=seed)[0] for seed in (1, 2)}
  1. After fixing the above two items (please add unit tests if feasible), the MAEs and Std Dev Errors probably will change. Do we have visualization for both items in the demo? What do they tell us now?

@alegresor

Copy link
Copy Markdown
Member
  1. generate_qmcpy_paths()in demos/GBM/gbm_code/qmcpy_util.py consistently returns a shape of (replications, n_paths, n_steps) even when replications=1.

For API consistency, when replications==1 we want to return (replications, n_paths, n_steps). When replications is None we want to return (n_paths, n_steps)

  • Please fix the documentation.
  • The file data_util.py uses the indexing qp_final = qp_paths[:, -1]. It should use qp_final = qp_paths[..., -1] or qp_final = qp_paths[:,:, -1] to extract the terminal values.

Prefer qp_final = qp_paths[..., -1] since we don't know if it will be 2 or 3 dimensional, see above comment.

@alegresor alegresor left a comment

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.

Defer to @sou-cheng-choi review on this

Comment thread demos/GBM/gbm_code/data_util.py Fixed
@larissensium

larissensium commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

1. The file data_util.py uses the indexing qp_final = qp_paths[:, -1]. It should use qp_final = qp_paths[..., -1] or qp_final = qp_paths[:,:, -1] to extract the terminal values.

I fixed qp_final but we only use collect_library_results() (where the bug was) to generate initial parameter-sweep results without replications. We then update these results with update_sweep_df() and specify the number of replications before generating plots so this bug should not affect MAE plots. We currently do not have Std dev error visualization.

2. generate_quantlib_paths are generating the same paths no matter how seed value changes. For example,

As far as I know Quantlib's Sobol implementation does not support randomization so changing the seed does not randomize the sequence. So we get the same paths despite changing the seed

@sou-cheng-choi

Copy link
Copy Markdown
Member

@larissensium

Please request for re-review from me and @alegresor when you feel ready.

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

@sou-cheng-choi sou-cheng-choi left a comment

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.

@larissensium

I have

  • merged in latest changes from develop branch
  • corrected some typos
  • made the paths from QuantLib independent with different seed values in quantlib_util.py
  • added unit tests

Please review and verify the changes.

@JiangruiKang JiangruiKang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The switch to Burley scrambling is the right way to obtain independent randomized-QMC replications, but one reproducibility defect remains at dimensions above the Jaeckel table. The current tests are green because they use n_steps=4; please cover the 32/33-dimensional boundary before merging.

# Jaeckel direction integers, so changing its `seed` does not create an
# independent replication. Burley2020SobolRsg applies a seeded Owen-style
# scramble; keep the underlying Sobol seed fixed and vary the scramble.
uniform_rsg = ql.Burley2020SobolRsg(dimension, 0, ql.SobolRsg.Jaeckel, seed)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Use a fixed nonzero underlying Sobol seed

QuantLib treats this second argument differently from the replication-specific scramble seed. Jaeckel direction integers are tabulated only through dimension 32; above that, SobolRsg uses this value to generate additional direction integers, and QuantLib defines 0 as a clock-selected random seed. Consequently, two calls with the same public seed can produce different paths when n_steps >= 33 (see lballabio/QuantLib#2732), even though the comment says the underlying seed is fixed. Please use a fixed nonzero value such as 42, vary only scrambleSeed, and add a regression test at n_steps=33 showing that equal scramble seeds reproduce exactly while different scramble seeds remain distinct.

@sou-cheng-choi sou-cheng-choi changed the title Minor fix for gbm_demo Fix GBM demo replication statistics and speed up the Gaussian transform Aug 18, 2026
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.

6 participants