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

renamed PDEs #344

Closed
bmcdanie opened this issue Sep 30, 2020 · 5 comments · Fixed by #368
Closed

renamed PDEs #344

bmcdanie opened this issue Sep 30, 2020 · 5 comments · Fixed by #368
Assignees
Labels
hardening non bugfix/performance improvements

Comments

@bmcdanie
Copy link
Collaborator

sync up fokkerplanck PDEs in C++ with renames to the MATLAB versions.

fokkerplanck1_4p1a -> fokkerplanck1_pitch_E(1) ,
fokkerplanck1_4p1b -> fokkerplanck1_pitch_E(2) ,
fokkerplanck1_4p2-> fokkerplanck1_pitch_C

@bmcdanie bmcdanie added the hardening non bugfix/performance improvements label Sep 30, 2020
@bmcdanie bmcdanie self-assigned this Sep 30, 2020
@hbrunie
Copy link
Collaborator

hbrunie commented Jan 11, 2021

@bmcdanie @dlg0 @lwonnell There is a difference between the description in the comment and the actual code I think.
The description on top shows:
df/dt == d/dz ( (1-z^2) df/dz ) code
But the description in the Matlab code shows df/dt == -d/dz ( (1-z^2)f. Notice the sign is different and f is not derived.
But in code the description is different. Same file but different comment. They should tell the same story I think. Which one is correct?

@hbrunie
Copy link
Collaborator

hbrunie commented Jan 11, 2021

Here is the branch corresponding to the changes I am making to solve this issue: branch

@dlg0
Copy link
Contributor

dlg0 commented Jan 11, 2021

fokkerplanck1_4p1a -> fokkerplanck1_pitch_E (case=1) 
fokkerplanck1_4p1b -> fokkerplanck1_pitch_E (case=2) ** this is a good adaptivity test case
fokkerplanck1_4p2-> fokkerplanck1_pitch_C
fokkerplanck1_4p3-> fokkerplanck1_pitch_R

It looks like the C++ does not have _4p1b so we will need to add it. It is a very minor variant of _4p1a and I'd like to add the case ability as we have in the matlab as a command line option in the C++, so this seems a good opportunity to do that?

@bmcdanie @hbrunie

@hbrunie
Copy link
Collaborator

hbrunie commented Jan 11, 2021

Yes I'm on it. The pr I'm building will contain a new option to choose the pde case (1 by default). So on long term for each pde we will be able to add new cases.
But @dlg0 you did not answer about the difference between the PDE in matlab and the one in C++.
The description on top shows:
df/dt == d/dz ( (1-z^2) df/dz ) code, but the description in the Matlab code shows df/dt == -d/dz ( (1-z^2)f.
Which one is the right one?

@dlg0
Copy link
Contributor

dlg0 commented Jan 11, 2021

It looks to me like in src/pde/pde_fokkerplanck1_4p1a.hpp the actual PDE coding matches the matlab, but the comment is a copy and paste error - so the matlab comment is correct, the C++ comment is incorrect. You can confirm this by noting only one grad operator, whereas anything of the form d/dz df/dz would have two.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hardening non bugfix/performance improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants