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

WIP: fix SROCK2 for NoiseWrapper noise #507

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

axsk
Copy link
Contributor

@axsk axsk commented Sep 30, 2022

fixes #505

@axsk axsk changed the title fix SROCK2 for NoiseWrapper noise WIP fix SROCK2 for NoiseWrapper noise Sep 30, 2022
@axsk
Copy link
Contributor Author

axsk commented Sep 30, 2022

Looking at the other uses of gen_prob I just realized that some methods don't care about W.rng and just use rand() anyways. I guess we want to use W.rng everywhere?

@axsk axsk changed the title WIP fix SROCK2 for NoiseWrapper noise WIP: fix SROCK2 for NoiseWrapper noise Sep 30, 2022
@ChrisRackauckas
Copy link
Member

What's the status of this?

@axsk
Copy link
Contributor Author

axsk commented Nov 28, 2022

Sorry for leaving this hanging, was on vacation and then forgot about it.
I think it is done.

The one thing I am not sure of (looking at it now) is that I am not using the oftype(W.dW, ... any more.
I don't know if that poses any problems.

@ChrisRackauckas
Copy link
Member

Add a test?

@axsk
Copy link
Contributor Author

axsk commented Nov 28, 2022

Well the canonical tests seem to be working. I think the oftype only comes to play when W.dW is of different type then rand(W.rng), and I wouldn't know when this is the case.

Edit: the omission of oftype is fine. c.f. the code comment below.

@axsk
Copy link
Contributor Author

axsk commented Nov 28, 2022

Or do you mean a test that the NoiseWrapper is actually respected?

@ChrisRackauckas
Copy link
Member

Or do you mean a test that the NoiseWrapper is actually respected?

That. Try solving it with a NoiseWrapper and seeing if it succeeds / fails. Also try other noise types like NoiseGrid`

@axsk
Copy link
Contributor Author

axsk commented Nov 28, 2022

Could you hint me at where to put these tests?

@ChrisRackauckas
Copy link
Member

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.

SROCK2() not working with NoiseWrapper
2 participants