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 with_custom for heating #492

Merged
merged 2 commits into from
Apr 3, 2024

Conversation

illorenzo7
Copy link
Contributor

The with_custom framework successfully modifies the heating by a custom reference file. However, the flag "adjust_reference_heating" has been set to .true. earlier (for any heating_type > 0) and so the user's choices will possibly be overwritten in the boundary conditions file.

This pull request changes adjust_reference_heating back to .false. if the user sets a custom heating profile using the with_custom framework.

@illorenzo7
Copy link
Contributor Author

I just realized this is failing one of the "with_custom" Jones benchmark case 0 tests. I may be being obstinate, but I think this test SHOULD have been failing before and (possibly) not falling after this pull request. I just confirmed that the simulation under

tests/custom_reference/augment/main_input

results in a heating function being zero because of the adjust_reference_heating issue. Presumably this case was meant to test what happens with nonzero heating in the Jones benchmark?

Unless I am missing something, I would then suggest replacing the file

tests/custom_reference/augment/Benchmark_Reports/00000200

which would presumably make this pull request pass the test.

@feathern
Copy link
Contributor

feathern commented Dec 1, 2023

I'll try to look at this in detail soonish. Might be a couple of days, but I'm aware of the PR.

@illorenzo7 illorenzo7 force-pushed the fix_withcustom_for_heating branch from 945c371 to 2e50d6a Compare December 5, 2023 23:03
@illorenzo7
Copy link
Contributor Author

Because of the latest approved pull requests, I removed the duplicate commit from #490 (the divide by N^2=0 pull request) and then rebased this off "main" to make the analysis of the specific change here more easily readable.

@illorenzo7 illorenzo7 force-pushed the fix_withcustom_for_heating branch from 2e50d6a to 74228db Compare December 6, 2023 22:28
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.

This is a good catch of a very subtle logical issue! Merging.

@feathern feathern merged commit cd0d556 into geodynamics:main Apr 3, 2024
5 checks passed
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