Implement quantile-based integration ranges in OneFactorCopula family - #2851
Open
Nityahapani wants to merge 1 commit into
Open
Nityahapani wants to merge 1 commit into
Nityahapani wants to merge 1 commit into
Conversation
Seven FIXME comments across onefactorcopula.cpp and onefactorstudentcopula.cpp
flagged the same problem: hardcoded ±10 integration limits for the Y-table
in performCalculations() and for the M/Z grids in cumulativeYintegral(), with
a note to derive them from a cutoff quantile instead.
The hardcoded limits are fragile:
- For fat-tailed Student-t distributions with low degrees of freedom, ±10
may not capture enough tail mass, biasing the computed probabilities.
- For all distributions, there is no connection between the grid bounds and
the actual tails of the marginals, making the quality of the approximation
opaque.
Fix: replace every hardcoded bound with a value computed at cutoff = 1e-6
(or 1e-7 for the Z moment check) using the appropriate inverse CDF:
OneFactorStudentCopula (both M and Z are Student-t):
ybound = InverseCumulativeStudent(nm)(1-c)*scaleM
+ InverseCumulativeStudent(nz)(1-c)*scaleZ
OneFactorGaussianStudentCopula (M Gaussian, Z Student-t):
ybound = InverseCumulativeNormal(1-c)
+ InverseCumulativeStudent(nz)(1-c)*scaleZ
OneFactorStudentGaussianCopula (M Student-t, Z Gaussian):
ybound = InverseCumulativeStudent(nm)(1-c)*scaleM
+ InverseCumulativeNormal(1-c)
checkMoments() Z grid: InverseCumulativeNormal(1-1e-7)
checkMoments() Y grid: use the already-computed y_.front()/y_.back()
which now reflect the quantile-based bounds
Also clarify the FIXME comment in conditionalProbability() explaining
why p < 1e-10 is guarded (numerical stability of inverseCumulativeY at
degenerate tail).
Adds #include for normaldistribution.hpp in onefactorcopula.cpp and
studenttdistribution.hpp in onefactorstudentcopula.cpp.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Seven FIXME comments across onefactorcopula.cpp and onefactorstudentcopula.cpp flagged the same problem: hardcoded ±10 integration limits for the Y-table in performCalculations() and for the M/Z grids in cumulativeYintegral(), with a note to derive them from a cutoff quantile instead.
The hardcoded limits are fragile:
Fix: replace every hardcoded bound with a value computed at cutoff = 1e-6 (or 1e-7 for the Z moment check) using the appropriate inverse CDF:
OneFactorStudentCopula (both M and Z are Student-t):
ybound = InverseCumulativeStudent(nm)(1-c)*scaleM
+ InverseCumulativeStudent(nz)(1-c)*scaleZ
OneFactorGaussianStudentCopula (M Gaussian, Z Student-t):
ybound = InverseCumulativeNormal(1-c)
+ InverseCumulativeStudent(nz)(1-c)*scaleZ
OneFactorStudentGaussianCopula (M Student-t, Z Gaussian):
ybound = InverseCumulativeStudent(nm)(1-c)*scaleM
+ InverseCumulativeNormal(1-c)
checkMoments() Z grid: InverseCumulativeNormal(1-1e-7)
checkMoments() Y grid: use the already-computed y_.front()/y_.back()
which now reflect the quantile-based bounds
Also clarify the FIXME comment in conditionalProbability() explaining why p < 1e-10 is guarded (numerical stability of inverseCumulativeY at degenerate tail).
Adds #include for normaldistribution.hpp in onefactorcopula.cpp and studenttdistribution.hpp in onefactorstudentcopula.cpp.