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

Merging Tg into the LLNL/develop #62

Merged
merged 34 commits into from
Jan 9, 2024
Merged

Conversation

ganjinzero1
Copy link
Member

The previous one was closed and this is the continuation

@holm10
Copy link
Collaborator

holm10 commented Dec 9, 2023

1/96 tests fails: D+C test case. It appears the fnrm is ever so slightly different between the reference and test, however larger than the tolerance. It's likely there are just some numerical noise propagating/multiplying with some of the changes introduced. This won't be an issue for a merge.

@holm10
Copy link
Collaborator

holm10 commented Dec 9, 2023

Alright: I've removed the parts relating to geometry and manufactured solutions as discussed. The Manufactured Solution terms should be done locally during development, and not make it into the "production" code. The 1D FT related changes should go into a separate PR to maintain some compartmentalization of the changes.

@holm10
Copy link
Collaborator

holm10 commented Dec 9, 2023

In order to demonstrate the functionality of the new coding relating to the Tg equation, we will need some examples.

Could you modify the input file of this example and reconverge it using the Tg equation? A couple of examples using different settings/switches for the Tg equation, if available, along with explanations in the comments should suffice.

Link to example: https://github.com/LLNL/UEDGE/tree/develop/pyexamples/input_example

@ganjinzero1
Copy link
Member Author

In order to demonstrate the functionality of the new coding relating to the Tg equation, we will need some examples.

Could you modify the input file of this example and reconverge it using the Tg equation? A couple of examples using different settings/switches for the Tg equation, if available, along with explanations in the comments should suffice.

Link to example: https://github.com/LLNL/UEDGE/tree/develop/pyexamples/input_example

Thanks for helping to get through the tests! I will work on building some test cases for the new features with solving Tg equations. The link you posted are referring to input files for a test case. So you mean we better build a case based on the input files in the link? or you mean I should build test cases in that directories

@holm10
Copy link
Collaborator

holm10 commented Dec 12, 2023

I suggest modifying the converged test case to include the new Tg-equation and reconverge it to serve as an example. If there are any optional switches that can used, having an example of them, too, would be useful. The actual cases could then either be added to the branch or uploaded to this PR as a .zip etc.: I'll need to integrate them into the test module manually the first time, regardless.

bbb/bbb.v Outdated Show resolved Hide resolved
bbb/bbb.v Outdated Show resolved Hide resolved
bbb/bbb.v Outdated Show resolved Hide resolved
bbb/bbb.v Outdated Show resolved Hide resolved
@ganjinzero1
Copy link
Member Author

Tg_withDrifts_andMol.tar.gz
Here is the example case turning on Tg equations for D0 and D2, with drifts on (forward B0 = 1.)

Menglong Zhao and others added 3 commits January 9, 2024 14:33
The variable scales the contribution of atom energy in the ion energy equation
- When Ti and Ta solved together, the switch should be set to 1
- When Ti and Ta solved separately, the switch should be set to 0
- Cases are allowed to run in faulty setups since the coefficient is scaleable to allow gradually turning it off
Copy link
Collaborator

@holm10 holm10 left a comment

Choose a reason for hiding this comment

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

  • Passes tests
  • Warnings added for switches
  • New features documented in .v files
  • Example case provided for new tests
    Approve PR and merge

elseif (istgcore(igsp) == 1) then # set to tgcore
yldot(iv)=nurlxg*(tgcore(igsp)*ev-tg(ix,0,igsp))/
. (temp0*ev)
elseif (istgcore(igsp) == 2) then
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not entirely clear to me what this option does on the core boundary, you'll need to add an explanation of this option to the description of tgcore.

bbb/bbb.v Outdated
@@ -324,6 +341,7 @@ vboost real /1./ +input #previously scaled eqp; no longer in use
cvgp real /1./ +input #Coef for v.Grad(p) ion/elec eng. terms
cvgpg real /1./ +input #Coef for v.Grad(pg) gas eng. terms
cfvgpx(1:nispmx) real /nispmx*1./ +input #Coefs for x components of v.grad(p) in ti-eq
cftiexclg real /1./ +input #Coef for including atom gas in Ti eq.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like this is the coefficient that decides whether to combine the atom and ion energy equation into the same or separate equations. If this is indeed the case, would it be prudent to include a conditional somewhere setting cftiexclg = 0 if istgon[0]=1, as one might risk doubly accounting for the terms otherwise?

@holm10 holm10 merged commit 672573f into LLNL:develop Jan 9, 2024
1 check 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