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

Fix None value in Pushforward #393

Merged
merged 4 commits into from
Mar 27, 2024
Merged

Fix None value in Pushforward #393

merged 4 commits into from
Mar 27, 2024

Conversation

campospinto
Copy link
Collaborator

there was a small bug in the Pushforward function, when no grid_local was passed for a mapping of Mapping type

I also added a unit test

@campospinto campospinto requested a review from yguclu March 21, 2024 12:39
@campospinto campospinto added the bug Something isn't working label Mar 21, 2024
Copy link
Member

@yguclu yguclu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

AFAIK the Pushforward class is only used by the PostProcessManager class (which always passes grid_local to it) and nowhere else in Psydac. So it is only tested indirectly in file test_postprocessing.py. So how did you find this bug?

psydac/feec/pushforward.py Outdated Show resolved Hide resolved
psydac/feec/tests/test_pushforward.py Show resolved Hide resolved
@campospinto
Copy link
Collaborator Author

Good catch!

AFAIK the Pushforward class is only used by the PostProcessManager class (which always passes grid_local to it) and nowhere else in Psydac. So it is only tested indirectly in file test_postprocessing.py. So how did you find this bug?

It is used in the code for conga polar splines

@yguclu
Copy link
Member

yguclu commented Mar 22, 2024

Good catch!
AFAIK the Pushforward class is only used by the PostProcessManager class (which always passes grid_local to it) and nowhere else in Psydac. So it is only tested indirectly in file test_postprocessing.py. So how did you find this bug?

It is used in the code for conga polar splines

I see... 😅... I must have decided to use it because it is quite a bit faster than the algorithms in pull_push.py.

The docstring of Pushforward says that the class constructor does not perform any checks on its arguments (probably because it was only used internally by the PostProcessManager class). Maybe we should add those checks now?

@yguclu yguclu changed the title Fix None value in Pushforward Fix None value in Pushforward Mar 27, 2024
Increase relative and absolute tolerances in `test_stencil_matrix_2d_serial_vdot`, which uses random numbers and often fails with complex values.
@yguclu
Copy link
Member

yguclu commented Mar 27, 2024

@campospinto I have increased the tolerance in the failing unit test. Hopefully our workflows will pass now.

@yguclu yguclu merged commit 9a7c8f7 into devel Mar 27, 2024
6 checks passed
@yguclu yguclu deleted the fix_pushforward_grid branch March 27, 2024 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants