-
Notifications
You must be signed in to change notification settings - Fork 21
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
Neutral gas temperature equation #47
Conversation
…ear the X-pt for a 1D flux tube ranging from the OMP to the outer target, I added a few lines so that it also works for the inner X-pt if one wants to use a full flux tube including both targets.
@holm10 @trognlien I would like to attach the two converged cases, one coverged with the V8 master version and the other with the develop version including the Tg modifications, that allow reviewers or users to run and see the differences. There shouldn't be differences though. However, I am not sure where I can attach those cases. |
Here are the files of the test example. The one named 'V8_master_test.tar.gz' is the case coverged with the latest V8 master version and the other is coverged with the develop version incorprating the modifications for Tg equations. One can load them and see the profiles. They should be the same :) PS this is a slab case (rectangular box).. I will work on a 2D tokamak case and load them here. Hopefully soon..:) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- There is a bug on L8234 of oderhs.m that causes the build to crash with newer compilers, could you fix that and recommit? Then the tests may run.
- There are quite a few commented-out commands added, could those be removed since they are not used? It makes the code easier to read.
- I am not sure about commenting out L4600 - L4609, see my comment
- It seems this commit also contains the structure for testing manufactured solutions. We can discuss at the next UESG meeting whether to include this in the codebase or not. I would, regardless, advocate having different PRs for the physics, and making another PR for the manufactured solutions, if we want to absorb it into the codebase
bbb/oderhs.m
Outdated
ivolcurgt = ivolcurgt + ivolcurg(igsp) | ||
#..zml manufactured solution | ||
if (ismanufactured(igsp) .ne. 1) | ||
. call s2fill (nx+2, ny+2, 0., pwrsorg(0,0,igsp), 1, nx+2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call causes an error with newer Fortran compilers. There is a mismatch between array shapes. The build error is:
gfortran -fPIC -ffixed-line-length-132 -DFPSIZE=8 -fdefault-real-8 -fdefault-double-8 -DISZ=8 -fdefault-integer-8 -fimplicit-none -fno-second-underscore -O3 -ftree-vectorize -ftree-vectorizer-verbose=0 -Ofast -DFORTHON -cpp -I/opt/hostedtoolcache/Python/3.9.17/x64/lib/python3.9/site-packages/numpy/core/include -c ../../bbb/oderhs.F
../../bbb/oderhs.F:8234:39:
8234 | . call s2fill (nx+2, ny+2, 0., pwrsorg(0,0,igsp), 1, nx+2)
| 1
......
10524 | call s2fill (nx+2, ny+2, 0., prdu, 1, nx+2)
| 2
Error: Element of assumed-shape or pointer array as actual argument at (1) cannot correspond to actual argument at (2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bill traced the error to newer compilers asserting the shape of the passed arguments. You need to pass the whole pwrsorg array rather than only the first index, e.g. pwrsorg(0:nx+1,0:ny+1,igsp)
bbb/oderhs.m
Outdated
call s2fill (nx+2, ny+2, 0., volpsor(0:nx+1,0:ny+1,igsp), 1, nx+2) | ||
call s2fill (nx+2, ny+2, 0., volmsor(0:nx+1,0:ny+1,igsp), 1, nx+2) | ||
#..zmlpossible bugs? | ||
#call s2fill (nx+2, ny+2, 0., volpsor(0:nx+1,0:ny+1,igsp), 1, nx+2) #..zml defined above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you remove the commented-out lines, if they are not used?
endif | ||
#..zml ion/molecule elastic collisions have been considered in | ||
#engbalg subroutine, so comment the following lines... | ||
# if(ishymol == 1) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that engbalg only accounts for the energy change of molecules due to elastic collisions, without considering the energy change of ions:
do igsp = 1, ngsp
do iy = j2, j5
iy1 = max(0,iy-1)
do ix = i2, i5
ix1 = ixm1(ix,iy)
reseg(ix,iy,igsp)= -( fegx(ix,iy,igsp)-fegx(ix1,iy, igsp)+
. fegy(ix,iy,igsp)-fegy(ix, iy1,igsp) )
. + segc(ix,iy,igsp)
reseg(ix,iy,igsp)= reseg(ix,iy,igsp) + vol(ix,iy)*
. eqpg(ix,iy,igsp)*(ti(ix,iy)-tg(ix,iy,igsp))+
. cftgdiss(igsp)*psorg(ix,iy,igsp)*tg(ix,iy,igsp)
enddo
enddo
enddo
It seems the energy residuals of the ions due to elastic scattering with molecules is only accounted for in these lines.
There were a few conficts in bbb/bbb.v and com/com.v. I have manually updated them and I think they look good now. However I am not sure what to do to let Github accept my current version of bbb.v and com.v. For example, Github warns that there are conflicts in bbb.v and when I clicked the button 'resolve conflicts' above, one example was shown below: <<<<<<<develop ======= ebind real [eV] /13.6/ +input However, what I want is to keep the original file, i.e. the second and third lines after "<<<<<<< develop". Shall I just remove what I don't need and mark it as resolved? |
@ganjinzero1 GitHub detected that you added cfdiss, but the sometimes formatting results in a conflict. To resolve this conflict, you just need to edit
to say
(i.e. remove the '====', '<<<<', '>>>>' and leave the coding you want to remain) and save the file and the conflict should be resolved. If there are more conflicts in the same files, you need to resolve them all in a similar manner. Let me know if this resolves it, or if I can help (I can see the conflicting files from the PR). |
We are closing this one and go to #62 |
We are closing this one and go to #62 |
The pull request is to merge the changes for solving neutral temperatures. I attached two example cases for comparison to show that the new modification doesn't affect cases that has been converged with the latest V8 master version.