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 Robin errors #615

Merged
merged 9 commits into from Dec 13, 2018
Merged

Fix Robin errors #615

merged 9 commits into from Dec 13, 2018

Conversation

guyer
Copy link
Member

@guyer guyer commented Dec 7, 2018

Volume integral of divergence
is equal to surface integral of normal flux
is approximately equal to sum over faces...
Divergence of a scalar field is unreliable.
Multiplying by surface normal seems to resolve that.
@guyer guyer requested review from wd15 and tkphd December 7, 2018 22:38
Copy link
Contributor

@tkphd tkphd left a comment

Choose a reason for hiding this comment

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

Volume integral in the divergence equations is now correctly represented.
Inclusion of surface normal in divergence of scalars is now correctly represented.
Minor formatting changes are requested, but this PR LGTM.

documentation/USAGE.rst Outdated Show resolved Hide resolved
documentation/USAGE.rst Outdated Show resolved Hide resolved
@guyer guyer requested a review from tkphd December 10, 2018 21:29
@guyer
Copy link
Member Author

guyer commented Dec 10, 2018

@tkphd Thanks for the suggested line wrappings. I've made some more pervasive changes to hopefully make this a bit clearer. Please take another look.

Copy link
Contributor

@tkphd tkphd left a comment

Choose a reason for hiding this comment

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

Nice documentation. Please define $f$ and $f_0$, then it looks good to merge.

documentation/USAGE.rst Show resolved Hide resolved
documentation/USAGE.rst Outdated Show resolved Hide resolved
documentation/USAGE.rst Outdated Show resolved Hide resolved
documentation/USAGE.rst Show resolved Hide resolved
documentation/USAGE.rst Outdated Show resolved Hide resolved
documentation/USAGE.rst Outdated Show resolved Hide resolved
@guyer guyer requested review from wd15 and tkphd December 13, 2018 00:29
@guyer
Copy link
Member Author

guyer commented Dec 13, 2018

@tkphd & @wd15: I made more than minor changes to address your comments. Please check.

Copy link
Contributor

@tkphd tkphd left a comment

Choose a reason for hiding this comment

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

Rendered output looks nice. Revised equations and language make the Robin condition much more clear.

@guyer guyer merged commit c0d3c7f into usnistgov:develop Dec 13, 2018
@guyer guyer deleted the fix_Robin_errors branch December 13, 2018 16:56
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.

None yet

3 participants