indentation issue/logic issue in core/Ions.py

[jslavin]

Hi Ken et al.,

I think I found a bug in core/Ions.py
It's in the if block that starts on line 149:
if setup:
if self.IonStr in chdata.MasterList:
...
else:
if verbose:
print(' ion %s not in masterlist, just various attributes, ionization, recombination rates'%(self.IonStr))

So, currently if setup is False and verbose is True it will print a message that the ion is not in masterlist. I think what is intended is for that block to be indented one level more. Then there is the question of the indentation of the next statements, which as currently written execute only if setup is False:
if np.any(temperature) is not None:
self.Temperature = np.atleast_1d(temperature)
self.Ntemp = self.Temperature.size
self.setupIonrec()
Was it intended that these be executed when ion is not in masterlist?
I ran into this because if you create an instance of ion with 'mg_1' as the ion, and then invoke the recombRate() method, you get an exception because the temperature is not defined.
I'm not sure what the proper behavior in this case is, but it seems clear that the current code is not correct.

Jon

[jslavin]

I think the piece of code I referred to needs to be changed such that if either setup is not True or the ion is not in the masterlist, then self.Temperature and self.Ntemp must be defined and setupIonrec() must be called. so here's my code snippet:

    if setup and self.IonStr in chdata.MasterList:
        self.IoneqAll = chdata.IoneqAll
        #  this needs to go after setting temperature and reading ionization
        #  equilibria
        #  needs to know self.NTempDens first
        self.argCheck(temperature, eDensity, pDensity, em)
        self.ioneqOne()

        self.setup()
    else:
        if verbose and (self.IonStr not in chdata.MasterList):
            print(f' ion {self.IonStr} not in masterlist, just various'
                    ' attributes, ionization, recombination rates')
        if np.any(temperature) is not None:
            self.Temperature = np.atleast_1d(temperature)
            self.Ntemp = self.Temperature.size
        self.setupIonrec()

I could make a pull request if you want.

Jon

[kdere]

Hi Jon, thanks for pointing this out. Yes, a pull request would be great. thanks, Ken

On 10/5/20 4:13 PM, Jonathan Slavin wrote: I think the piece of code I referred to needs to be changed such that if either setup is not True or the ion is not in the masterlist, then self.Temperature and self.Ntemp must be defined and setupIonrec() must be called. so here's my code snippet: |if setup and self.IonStr in chdata.MasterList: self.IoneqAll = chdata.IoneqAll # this needs to go after setting temperature and reading ionization # equilibria # needs to know self.NTempDens first self.argCheck(temperature, eDensity, pDensity, em) self.ioneqOne() self.setup() else: if verbose and (self.IonStr not in chdata.MasterList): print(f' ion {self.IonStr} not in masterlist, just various' ' attributes, ionization, recombination rates') if np.any(temperature) is not None: self.Temperature = np.atleast_1d(temperature) self.Ntemp = self.Temperature.size self.setupIonrec() | I could make a pull request if you want. Jon — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <https://secure-web.cisco.com/1ffGeobM4WIlSFF6ZU5cYvadAUkggQ1OcwMha-XIIWyyQMnN1bSTFot2cqJU5gUYKUIk68cpmb--NzG9E2bgZRUBpQmup2Xx5rRZ8gVzjLwxTgUgW8UNO6eH5e5B77U4_MxrnTO-63wu9QEs1JdAsfNpXZFaiOJEH0N_nn4FsdK59g8EddVM12pqxIapmXyyvNMDxMLi9BDcp7xpX0IOnH60NbyApevpV2UK6SUVr3sPiw4vhLeNXxPp0QPesoAkXZUiozPiNrb4IObrIhjq7qrA2RkKKJPdB431KaWrp_wmkdZZxduVqOF9ksL8r8ptZkBjh4Ai0n5DmsK5u0GeLcavILZoUBMplqD6f92grpco1bZRGClXMfN5zvnTF3g8OBRK2_5P3h_uJGKCK07Cok7ohYN7byiF6Q8bBTC_Y0d_33SyTIMz8b-vlJQ1EDaM4/https%3A%2F%2Fgithub.com%2Fchianti-atomic%2FChiantiPy%2Fissues%2F286%23issuecomment-703863181>, or unsubscribe <https://secure-web.cisco.com/1m00qoVt6tEZCl9IXOD7qoUb1_mGrMbubesuPsz_pnMC5_MMEAf0MZI5CCiavDS9u2HSQmlQD7GqwFWQTpfiuGotLI8cVmBQbS5euZM0OPB9EgCRrG0nOAB285S7DcsslkaAZGE1e-EibcmJURukC4dS18ycz4j! u2wDhFqh9q1o07H4ZCqo3NGHY-xQX1BfHJvkloEOwFoG8gMZlz_XLDfRdu3mM7icjakPip9ziUjhu-gvNafqJC_iBODXe9mKaz3uXC4UoFqAxF3RvAG5-Gp7zRCOM7HrZef66iyn3xkQKMMSMVUnK_IUmASIECF1kXM7bJHRGuNZW9dzzPEJEeHXkQRyPN23ZG5ZxmH-AcIpUxDJKl5-9wDmeebG_FRxkZGs5qBOdjyrFDTv2ap5PqSt3rDSryGtD0mfG65hNVnPic7_8mP00WC-tgNBSCT2YJ/https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAFUQVWWP3MBVJ6XZZT37I3SJISGXANCNFSM4SFCEI5A>.

-- Kenneth P. Dere Research Professor of Solar Physics Department of Physics and Astronomy George Mason University [email protected]