Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] bugfixes to QPD distributions - QPD_U, QPD_S #194

Merged
merged 16 commits into from
Apr 19, 2024

Conversation

fkiraly
Copy link
Collaborator

@fkiraly fkiraly commented Jan 30, 2024

Re-enables tests for QPD_U, QPD_S skipped in #193 and attempts to fix failures in QPD distributions.

it appears that #190 failures are resolved - fixes #190.

Further QPD_B seems to fail at mean and var calculations, this is a separate bug and raised as #257. As a mechanical consequence, QPD_B is left skipped in the tests.

@fkiraly fkiraly added the maintenance Continuous integration, unit testing & package distribution label Jan 30, 2024
@fkiraly fkiraly changed the title [MNT] re-enable CyclicBoosting and QPD tests once #189 failures are resolved [MNT] re-enable CyclicBoosting and QPD tests once #190 failures are resolved Jan 30, 2024
@fkiraly fkiraly changed the title [MNT] re-enable CyclicBoosting and QPD tests once #190 failures are resolved [MNT] re-enable QPD tests once #190 failures are resolved Jan 30, 2024
@fkiraly
Copy link
Collaborator Author

fkiraly commented Apr 18, 2024

@setoguchi-naoki, @FelixWick, I have been trying to fix the open bug after re-enabling the tests, it seems like there are some issues with the internal indexing.

I'm not sure what format of alpha, qc_low etc the interna code assumes, could you help me? I was assuming, flat numpy array (1D), but that does not seem to work.

@FelixWick
Copy link

alpha: float,
qv_low: Union[float, np.ndarray],
qv_median: Union[float, np.ndarray],
qv_high: Union[float, np.ndarray],

@fkiraly
Copy link
Collaborator Author

fkiraly commented Apr 19, 2024

I mean, internally. You do this: params[idx] = np.array([p]), so alpha becomes a 1D array, no?

@fkiraly fkiraly added bug module:probability&simulation probability distributions and simulators and removed maintenance Continuous integration, unit testing & package distribution labels Apr 19, 2024
@fkiraly fkiraly changed the title [MNT] re-enable QPD tests once #190 failures are resolved [BUG] bugfixes to QPD distributions Apr 19, 2024
@fkiraly
Copy link
Collaborator Author

fkiraly commented Apr 19, 2024

Thanks, this seems to have helped.

Now, QPD_B is failing internally, it is substituting 0 or 1 into a logit, which produces nans.
Any ideas where or why, @FelixWick, @setoguchi-naoki?

It should not substitute the bounds, because they will always be nan, so where is that coming from?

@fkiraly fkiraly changed the title [BUG] bugfixes to QPD distributions [BUG] bugfixes to QPD distributions - QPD_U, QPD_S Apr 19, 2024
@fkiraly
Copy link
Collaborator Author

fkiraly commented Apr 19, 2024

FYI, it seems the bugs we were targeting are now resolved.

I have opened a bug issue here, to track that separately: #257

@fkiraly fkiraly merged commit d37092f into main Apr 19, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug module:probability&simulation probability distributions and simulators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] QPD distribution and CyclicBoosting API non-compliance
2 participants