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

enabling PBC and Regularization at the same time increases error #6

Open
mumairsadiq opened this issue Nov 13, 2024 · 5 comments
Open

Comments

@mumairsadiq
Copy link

Regularization and PBC work well when only one of them is selected.

Results are incorrect when both of the options are selected,

@mumairsadiq
Copy link
Author

Regularization and PBC work well when only one of them is selected.

Results are incorrect when both of the options are selected,

Hi @jooooow , I have found the error I think,

Currently, I have tested some examples where the fix works well. I will test it further and then create a pull request.

@mumairsadiq mumairsadiq changed the title enable PBC and Regularization at the same time increases error enabling PBC and Regularization at the same time increases error Nov 13, 2024
@jooooow
Copy link
Contributor

jooooow commented Nov 13, 2024

@mumairsadiq
Yes you're right.
As written in the README.md,

WARNING : currently, enable PBC and Regularization at the same will increase the error

When regularization is used, the cells are extended to contain some extra regularized bodies.
These regularized bodies are used to compute the multipole and local expansion, as well as the non-regularized bodies.
The existance of regularized bodies ensures the correctness of regularization, but also introduce large error when i>0.
When i>0, cells are copied and put arround the original box recursively.
Consequencely, some extra regularized bodies will also be added to the outermost edge, which result in a large error.

Recently, I'm working on fixing this problem(on branch 3-ewald-fmm) and found it's not an easy job.
But ofcause, i will apprecriate it if you can fix it.

@mumairsadiq
Copy link
Author

Actually, I tried a tactic,

So far it is producing correct result when we run with combined regularization and PBC setting

Let me test it futher and see if the solution works correct for all scenarios.

@jooooow
Copy link
Contributor

jooooow commented Nov 13, 2024

I invited you to a previous repo.
You can take a look at issue-1 for some reference.

@mumairsadiq
Copy link
Author

Sure, thank you.

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

No branches or pull requests

2 participants