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

Neutral gas temperature equation #47

Closed
wants to merge 16 commits into from

Conversation

ganjinzero1
Copy link
Member

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.

Menglong Zhao and others added 7 commits June 13, 2023 15:17
@ganjinzero1 ganjinzero1 marked this pull request as ready for review August 24, 2023 21:10
@ganjinzero1
Copy link
Member Author

@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.

@holm10
Copy link
Collaborator

holm10 commented Aug 24, 2023

You can use the comments to do so: just drag the file(s) (preferably .zips/tarballs) into the comment box:
image

@holm10 holm10 changed the title Develop Neutral gas temperature equation Aug 24, 2023
@ganjinzero1
Copy link
Member Author

ganjinzero1 commented Aug 24, 2023

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 :)
V8_master_test.tar.gz
V8_develop_test.tar.gz

PS this is a slab case (rectangular box).. I will work on a 2D tokamak case and load them here. Hopefully soon..:)

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.

  • 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)
Copy link
Collaborator

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)

Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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.

@ganjinzero1
Copy link
Member Author

ganjinzero1 commented Oct 19, 2023

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
cfdiss real /1./ # fraction of neutrals that are from dissociations.
ebind real [eV] /13.6/ +input

=======

ebind real [eV] /13.6/ +input
.>>>>>>> develop

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?

@holm10
Copy link
Collaborator

holm10 commented Oct 19, 2023

@ganjinzero1 GitHub detected that you added cfdiss, but the sometimes formatting results in a conflict. To resolve this conflict, you just need to edit

<<<<<<<develop
cfdiss real /1./ # fraction of neutrals that are from dissociations.
ebind real [eV] /13.6/ +input

=======

ebind real [eV] /13.6/ +input
.>>>>>>> develop

to say

cfdiss real /1./ # fraction of neutrals that are from dissociations.
ebind real [eV] /13.6/ +input

(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).

@ganjinzero1
Copy link
Member Author

We are closing this one and go to #62

@ganjinzero1
Copy link
Member Author

We are closing this one and go to #62

@ganjinzero1 ganjinzero1 closed this Nov 8, 2023
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.

3 participants