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

Add remake ability for NoiseProcess #112

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

Conversation

dannys4
Copy link

@dannys4 dannys4 commented Aug 16, 2022

This should allow the user to remake a NoiseProcess.

@dannys4
Copy link
Author

dannys4 commented Aug 16, 2022

Willing to take criticism on this method of doing it-- you could probably get close by taking advantage of the generic definition of remake in DiffEqBase, but AFAIK you'd have to remove the defined constructor for NoiseProcess and expose the original constructor, which could break a lot of downstream stuff for a tiny tweak. I felt more comfortable adding a few specific, but extensible utilities prescribed for the NoiseProcess type. If you disagree or find this dubious, feel free to close without merging.

@dannys4 dannys4 marked this pull request as ready for review August 16, 2022 21:59
@dannys4 dannys4 changed the title Add SciMLBase.@add_kwonly for NoiseProcess Add remake ability for NoiseProcess Aug 16, 2022
@dannys4
Copy link
Author

dannys4 commented Aug 16, 2022

Also, I'd be happy to add a test if you're content with the implementation and would like me to.

@mschauer
Copy link
Contributor

mschauer commented Aug 17, 2022

I think I would test that copy works as intended (same data, different memory)

@ChrisRackauckas
Copy link
Member

This is fine, just needs tests.

@ChrisRackauckas
Copy link
Member

Xoshiro is only defined v1.7 and above I think, so the test needs a v1.6 version

@rmsrosa
Copy link
Contributor

rmsrosa commented Nov 4, 2022

What is the status of this?

Notice that copy for every type of noise has been recently implemented in #127 and can be useful.

Question? Is remake assume to keep the same field types and just change the values or does it (in general) allow changing the type as well?

@rmsrosa
Copy link
Contributor

rmsrosa commented Nov 4, 2022

Question? Is remake assume to keep the same field types and just change the values or does it (in general) allow changing the type as well?

Nevermind. I just checked that remake for an ODEProblem allows changing the type of fields. But the implemented copy, and any sound implementation of similar keeps the type. So, if we want to allow changing the type, we would need to implement remake directly.

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.

4 participants