search

PrincetonUniversity / SPEC

The Stepped-Pressure Equilibrium Code, an advanced MRxMHD equilibrium solver.
https://princetonuniversity.github.io/SPEC/
GNU General Public License v3.0
25 stars 6 forks source link

Igeometry=2 disabled in current version #52

Closed zhisong closed 6 years ago

zhisong commented 6 years ago

I tried to run a fixed-boundary calculation for cylindrical coordinate (Igeometry=2). I set Lfreebound=0, meaning this is not a free boundary. Currently the code will return an error, stating that "free-boundary calculations not yet implemented in cylindrical geometry", though I am not using free-boundary at all.

I suspect an if condition should be added to "preset.h" line 1146, to skip the check for fixed-boundary. I am just not sure if the variables below 1146 will also be used in fix-boundary (very unlikely).

jloizu commented 6 years ago

I see what you mean. And I agree that adding an if check would fix the problem, provided that these variables (Dxyz, Nxyz) are not used in fixed boundary (I think indeed they are not because they describe the computational boundary). Can you first check that these are indeed never used in fixed-boundary? If so, then I would suggest that you add this IF statement in preset.h in a new temporary branch, so we can check that SPEC runs fine in fixed/free boundary test cases.

zhisong commented 6 years ago

Done as suggested. A simple grep shows Dxyz, Nxyz and Jxyz only appear in casing.h and bnorm.h, and therefore should be irrelevant to fixed-boundary.

jloizu commented 6 years ago

Great. I am going to test if the branch compiles and runs well on my end, and then you can merge the branch into master. Also, while doing that, can you (first) add a test input file for fixed-boundary, cylindrical geometry? That can be done in the folder InputFiles/TestCases/ and with the format G2VxxLyFi.001.sp where xx is the number of volumes and y is the Lconstraint number. Also one should add a file G2VxxLyFi.001.info with some description of the test case.

jloizu commented 6 years ago

Works for me. I wrote some review on the pull request. I think you can close this issue after merging.