search

ORNL-Fusion / aorsa

All ORders Spectral Algorithm (The Original)
MIT License
11 stars 2 forks source link

Iteration over species / number of considered species in an AORSA run #26

Closed AreWeDreaming closed 1 year ago

AreWeDreaming commented 4 years ago

I am working on the AORSA GUI and I noticed an inconsistency with the number of species considered in AORSA.

It seems that all iterations over species are hardcoded in AORSA, i,e, split into 6 separate lines always assuming 6 separate species. Could this be a potential performance issue? It seems that iterations over species are only performed in aorsa2dMain.F, so replacing he separate lines by proper for-loops with a variable specifying the number of considered species might not involve much work. If you disagree with this, let me please know so I have a better feel for the scope of the aorsa restoration project.

S_nspec seems to be already in place for the number of species iterations but it is never used. Also, Cornwall's input file is inconsistent with a total of 4 thermal (S_NSPEC_TH = 4) + 2 (default for n_spec_nm_max) fast ion species being declared, but S_nspec specified to be 2 and a total of 5 profiles supplied.

I just wanted to let you know about this.

cornwalllau commented 4 years ago

Thanks Severin for noticing this.

I'm not sure if the hardcoding is a performance issue, but it is certainly a readability issue. After some recollection, what I "think" is happening in the input file is that there are 6 species in the input. Note that 800 after many of the variables. I think that's because 181 is a hardcoded variable (nrho_max in profile_mod.f90). There are 101 interesting input points + 80 0's for 181 points. I don't understand why the code breaks if you don't put in 181ish elements in each profile. However, for the 5th species, it is hardcoded as 2620. I think this is done to specify the 6th species to be all 0.

This is obviously a readability issue that should be corrected.

AreWeDreaming commented 4 years ago

However, for the 5th species, it is hardcoded as 262_0. I think this is done to specify the 6th species to be all 0. Not quite true. It is another 80 to fill species no. 5 and then another 181 to fill species no. 6 since 6 species are hardcoded, and I mean really hardcoded as in separate lines for every species, in aorsa. Your input file is actually, strictly speaking, incorrectly putting 262 zeros there, but since this is the last species you can put there anything greater than 261 and it will be fine.

cornwalllau commented 4 years ago

That sounds very plausible and it wouldn't surprise me if I had a discussion with Fred on this. Clearly, we didn't do anything to solve it...

jcwright77 commented 4 years ago

Just to point out that all of the s_ variables were added to work with the plasma state in the IPS project. It is unclear if they were fully adopted internally in the code. There are duplicates in the namelist for the same functionality from the earlier aorsa usage. Its all very confusing.

The handling of species profile inputs in very error prone and requires some 'expert' level of understanding. I'd support cleaning this up to make it more robust. Whether we hold it to be backwards compatible is up for discussion. That does tend to make the input processing more complex. The zero filling should not be necessary in the namelist input.

As a first step, we should document the handling and use of the IPS state variables internally in aorsa and move towards a unified input approach. Someone (eg David and Don) should verify if the IPS state input namelist is still relied on by ATOM/IPS so we don't break anything.

-john

I am working on the AORSA GUI and I noticed an inconsistency with the number of species considered in AORSA.

It seems that all iterations over species are hardcoded in AORSA, i,e, split into 6 separate lines always assuming 6 separate species. Could this be a potential performance issue? It seems that iterations over species are only performed in aorsa2dMain.F, so replacing he separate lines by proper for-loops with a variable specifying the number of considered species might not involve much work. If you disagree with this, let me please know so I have a better feel for the scope of the aorsa restoration project.

S_nspec seems to be already in place for the number of species iterations but it is never used. Also, Cornwall's input file is inconsistent with a total of 4 thermal (S_NSPEC_TH = 4) + 2 (default for n_spec_nm_max) fast ion species being declared, but S_nspec specified to be 2 and a total of 5 profiles supplied.

I just wanted to let you know about this.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/ORNL-Fusion/aorsa/issues/26

dlg0 commented 4 years ago

the aorsa restoration project I like this name :)

Yes, it is all very confusing and the code base is in need of some love. Looping over species number would be an obvious improvement (although not a performance improvement).

In regard to the s_ variables, I'm not sure. I'll have to talk with @batchelordb about it, and perhaps test the IPS wrapper for AORSA too.