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

guarded against dividing by N^2=0 #490

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

illorenzo7
Copy link
Contributor

For reference_type = 5, when the buoyancy frequency N^2 was rescaled, this was causing problems with division by 0. I guard against this now with if-statements.

@illorenzo7 illorenzo7 force-pushed the fix_divide_by_0_nsquared branch from f6a8f47 to 4471ac2 Compare December 1, 2023 03:10
@feathern
Copy link
Contributor

feathern commented Dec 1, 2023

This looks good, but I have a quick question. Why is there duplicated code starting on line 760 (the same three lines are repeated), but then inside of the if statement, there is no duplication. Was the original duplication in error, or did it serve a purpose. Specifically, I'm talking about:

        Call Integrate_in_radius(nsquared,norm)
        norm = four_pi*norm/shell_volume
        nsquared = nsquared/norm

@illorenzo7
Copy link
Contributor Author

Hmm, it looks like my original code had the duplication, which was a mistake on my part, as far as I can tell. I must have removed the duplicate block when I surrounded it by the if-statement. I don't think removing the duplicate block should affect anything.

Copy link
Contributor

@feathern feathern left a comment

Choose a reason for hiding this comment

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

Looks good. Approved.

@feathern feathern merged commit c2916bc into geodynamics:main Dec 5, 2023
8 checks passed
@illorenzo7
Copy link
Contributor Author

Thanks @feathern ! Good catch, I forgot I put those debugging statements there.

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.

2 participants