#114 greatly improved the Continuum module, but broke compatibility with other modules that call it, including spectrum and radLoss. Until these things are fixed, these modules will be broken and the tests will fail.

Also, the Continuum documentation needs improving. This issue is mostly so I don't forget to do these things.

TODO List

• Add loss function methods for free-free, free-bound
• Fix continuum tests
• Fix other modules which use continuum module
• Documentation

In chianti.core.Ion.py, the gofnt() contribution function currently shows a plot of relative emissivity vs temperature, prompts the user to select a top line, and then plots the specific contribution Gofnt for that ion vs temperature. In the middle of this process, self.Gofnt is defined.

Could the plotting of this function be optional? Then, the attribute self.Gofnt (defined with the temperature, electron density, contribution function, index, and wavelength still) could be obtained by giving the same information at the function call. Implementing this would be beneficial for finding the contribution of multiple ions at a time.

Also, the class ionWeb in this file has the function gofntSelectLines. I am interested in having the self.toplines available as in this function, but was unable to access the class.

for example

k4=ch.ion('k_4', 1.e+5, 1.e+9, setup=0)
k4.setup(alternate_dir='.')

gives
elvlc file does not exist: /data2/svn/chianti/dbase/trunk/k/k_4/k_4.elvlc

it should look in the local directory (pwd/cwd) to find the files

The ipython_notebooks directory should be removed. The examples in the quickstart notebook should be moved to the docs, perhaps rendered with sphinx gallery. See also #145

The _IonNew class should be moved from the master branch, either into a pull request against master or just a on a new branch on a fork. In general, we should avoid having incomplete code or works-in-progress in the master branch.

I just installed ChiantiPy by cloning from this repo and running python setup.py install. Operating system is Windows 10 running Anaconda Python 2.7.12.

I set the environment variables and got ChiantiPy.core to import, but got all sorts of attribute errors when running the test lines. Finally realized that I was getting a warning on import, "found more than one ioneq file." When I removed the chianti.ioneq file inside ioneq/, it broke ChiantiPy. I went into ioneq/PaxHeader.23462 and removed that chianti.ioneq file, and now everything works fine.

So, it works. But strange...

This isn't so much an issue with the codebase, but rather with how ChiantiPy is distributed.

In addition to this main repository on GitHub, ChiantiPy is still available via SourceForge as well as through a seemingly "stale" GitHub repo (doesn't seem to have been updated in the past year or so). It is also available via pip install ChiantiPy.

The problem here is that, as far as I can tell, each of these sources holds a different version of ChiantiPy. I think this has the potential to really confuse users, especially those that may be hesitant to move to Python (from IDL) in the first place.

It seems that most development of ChiantiPy has now moved to this GitHub repo. I would propose removing the version on SourceForge (or at least stop linking to it) as well as the stale version that is on GitHub. Then the most current and stable version of this repo can be pushed to PyPI so that users can obtain it via pip install as well. In this way, the code is being distributed from a single source and there is less of a risk of a user accidentally downloading it from the "wrong" place.

@Cadair Any thoughts on setting up a pipeline from GitHub to PyPI? How does SunPy do this?

The CHIANTI database webpage also still links to the SourceForge page though it looks like there is a link to the GitHub page as well. I would propose linking only to this GitHub repo and the Read the Docs page. It might also be helpful to current and future users to make an announcement about the Read the Docs page and encourage them to report issues/request features via GitHub.

@kdere @ehsteve @Cadair Thoughts on this?

Instead of bringing up several dialogs and plots, ion.gofnt() should just return to the user (or set the ion.Gofnt dictionary) a N(temperature/density) by N(wavelength) array with the associated contribution function values. The user can then decide which transitions are the important ones. Since the wavelength values are returned to the user in the ion.Gofnt dictionary anyway, it should still be easy to select particular transitions based on their wavelength values.

I did the upstream pull and edited the files that had the conflicts and added them, Then did a commit. All went well except it say that my fork is now 1 commit behind the upstream.
How do I find and fix that?

Currently, the Quick Start sections of the docs rely on static pngs and code snippets that need to be updated and uploaded manually. As a result, they easily fall out of date (e.g. the PNGs in the docs are still leftover from the old SourceForge docs).

Instead, we should use the matplotlib plot directive such that the code snippets are always up to date and produce the plots in line each time the docs are generated. This ensures that the included plots are not out of date with the code itself. As an example, see how SunPy has implemented this. It is pretty straightforward.

Additionally, we should use sphinx-gallery to provide some nice standalone examples of the capabilities in ChiantiPy and to provide links to downloadable notebooks and scripts. As an example, see the SunPy docs.

(This will probably be a rather lengthy project so if there is a consensus here, I would suggest moving this to our list of long term projects in the wiki.)

The ionWeb appears to be unused and outdated. I think it can be safely removed.

this got messed up. chiantipy is a fairly tightly integrated packed so that moving all the directories around can cause a big mess.

I do not understand the need to actually install ChiantiPy. I myself do not and I have noticed others also do not install but have to come up with a version.py file. I think we need to maintain the possibility of just putting ChiantiPy in the Python path and explain why installing ChiantiPy buys the use something

@kdere moving this possible mathjax issue to a new thread. Quoting from #16,

I have been running in circles trying to figure out how to include mathjax (or imgmath) for my local doc builds. I think I have some good leads to follow but if anyone has some good ideas I would appreciate them.

Are equations in restructuredText not showing up when you build the docs with python setup.py build_sphinx? For example, I've used the .. math:: directives in the docstring ChiantiPy.tools.util.qrp.

I don't have any problems when building locally on my machine. astropy-helpers actually already includes the needed math extensions as seen here, lines 150-155. So I don't think we should have to worry about this...

Which browser are you viewing the docs in? I've had good results with Chrome, Firefox, and Safari.

These should be consolidated into a single object. Any changes needed for running in an IPython notebook/session should be able to be configured with command line arguments.

PEP8 is the default style-guide for Python code. Recently SunPy has made a push to enforce PEP8 conventions in all PRs, with the eventual goal of having the whole codebase PEP8-compliant.

As ChiantiPy attracts more users and developers, we should make an effort to enforce consistent styling across the whole codebase. PEP8 is the standard so it makes sense to adopt this style. Other scientific packages (e.g. numpy, scipy, astropy) also adhere to PEP8. This will help to improve readability as well as usability.

This is not a high-priority issue, but it may be a good idea to try to implement an agreed upon style-guide as new PRs come in. That way, the main work will be in fixing the styling of older code.

For a temperature array between 1e4 and 1e7 K spaced evenly in log space, the p2eRatio() method in the ion class returns incorrect proton to electron density ratios, particularly for temperatures below 10^5, according to those answers returned by the equivalent IDL CHIANTI routine proton_dens. Ratios for T>10^5 K seem to be in pretty good agreement (i.e. within ~.1%).

It's not clear to me exactly what is causing this issue, but it could have something to do with the repeated interpolations that are done in p2eRatio. The code snippet below (using Eq. 7 of Young et al., 2003) seems to give the correct answers (when compared with the IDL output):

import numpy as np
import scipy.interpolate
import ChiantiPy.tools as ch_tools

ioneq = ch_tools.io.ioneqRead(ioneqname='chianti')
abundance = ch_tools.io.abundanceRead(abundancename='sun_photospheric_2007_grevesse.abund')

abundance = abundance['abundance'][abundance['abundance']>0]
denom = np.zeros(len(ioneq['ioneqTemperature']))

for i in range(len(abundance)):
    for z in range(1,i+2):
        denom += z*ioneq['ioneqAll'][i,z,:]*abundance[i]

p2eratio = abundance[0]*ioneq['ioneqAll'][0,1,:]/denom
temperature = np.logspace(4,7,100)
f_interp = scipy.interpolate.interp1d(np.log10(ioneq['ioneqTemperature']),p2eratio,kind='cubic')
p2eratio_interp = f_interp(np.log10(temperature))

The equivalent IDL command is:

!abund_file = '/path/to/chianti/dbase/abundance/sun_photospheric_2007_grevesse.abund'
temperature =  findgen(100)*(7-4)/99+4
p2eratio = proton_dens(temperature) 

The free-bound radiative losses in Chianti IDL and ChiantiPy do not match. This is following the refactor of the Continuum module in #117 and #114. I do not know if they matched before. A plot of the Python and IDL results in shown below. The free-free results do not match exactly but are pretty close.

As noted by @kdere in #117 ,

I am pretty sure that CHIANTI will be going with the FB/radiative recombination losses by Badnell and co. so it would be interesting in finding the differences but not perhaps a most immediate goal.

I've done a fairly detailed comparison between the calculation as currently implemented in ChiantiPy and the IDL code fb_rad_loss.pro and cannot see any obvious differences. Going back to the old continuum calculations and doing another comparison would probably be helpful.

the final free-bound calculation comes together inside a try: statement so it is not obvious what is happening. Looking at it a bit it comes from trying to multiply 2 or 3 arrays together that have the wrong dimensions for this operation. One of the arrays seems to the the EM but that is not always the sticking point.

When calling mspectrum() on macOS (10.12.6, python3.6) the NotImplementedError is raised.
It seems to be a multiprocessing module dependent error, since calling the qsize() method might fail under macOS.

NotImplementedError                       Traceback (most recent call last)
<timed exec> in <module>()

/opt/local/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/ChiantiPy-0.7.dev534-py3.6.egg/ChiantiPy/core/Mspectrum.py in __init__(self, temperature, eDensity, wavelength, filter, label, elementList, ionList, minAbund, keepIons, abundance, doContinuum, allLines, em, proc, verbose, timeout)
    162                 ionWorkerQ.put((akey, temperature, eDensity, wavelength, filter, allLines, abundance, em, doContinuum))
    163         #
--> 164         ffWorkerQSize = ffWorkerQ.qsize()
    165         fbWorkerQSize = fbWorkerQ.qsize()
    166         ionWorkerQSize = ionWorkerQ.qsize()

/opt/local/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/multiprocessing/queues.py in qsize(self)
    115     def qsize(self):
    116         # Raises NotImplementedError on Mac OSX because of broken sem_getvalue()
--> 117         return self._maxsize - self._sem._semlock._get_value()
    118 
    119     def empty(self):

NotImplementedError: 

As an example, taken from the Quick Start page, this code leads to an IndexError (single process):

temp=2.e+7
dens=1.e+9
wvl = 1. + 0.002*np.arange(4501)
s3 = ch.spectrum(temp, dens, wvl, filter = (chfilters.gaussian,.015),doContinuum=1, em=1.e+27,minAbund=1.e-5,verbose=0)

Output:

Traceback (most recent call last):
  File "coronal_spectrum.py", line 11, in <module>
    doContinuum=1, em=1.e+27,minAbund=1.e-4,verbose=0)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/ChiantiPy-0.7.dev534-py3.6.egg/ChiantiPy/core/Spectrum.py", line 163, in __init__
    FF.calculate_free_free_emission(wavelength)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/ChiantiPy-0.7.dev534-py3.6.egg/ChiantiPy/core/Continuum.py", line 176, in calculate_free_free_emission
    gf_itoh = self.itoh_gaunt_factor(wavelength)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/site-packages/ChiantiPy-0.7.dev534-py3.6.egg/ChiantiPy/core/Continuum.py", line 224, in itoh_gaunt_factor
    gf += (itoh_coefficients[i,j]*(t**i))[:,np.newaxis]*(upper_u**j)
IndexError: invalid index to scalar variable.

Using arrays for the temperature values works fine. The problem is in line 224 in Continuum.py, were the np.newaxis expression expects the left value to be array-like, whereas a single temperature input leads to an Index Error.

In the CHIANTI IDL routines, EMISS_CALC.pro calculates,

with units of erg s^-1. The closest function I can find to this is the emiss method of the ion class which returns a quantity with units ergs cm^-3 s^-1 sr^-1 in the emiss field of the Emiss data structure. What exactly is this quantity? I assume that the sr^-1 just comes from the 1/4pi factor. Will dividing by N(X+m) (and multiplying by 4pi) give the same output as EMISS_CALC?

It would be helpful if there was some function like download_database() which goes out and installs the database in some default but user-configurable directory. Chiantipy should also keep track of what version of the database is has installed (if any) and issue a error if the version is not compatible with a new database file or if no database file is found (and can suggest how to download it).

Most of the core classes import tools.data which automatically reads info from the database. That means anytime import ChiantiPy.core is called, the database has to be installed or an IOError is thrown. This is a problem primarily with Read the Docs as very often the download of the database fails or causes the build to timeout.

One alternative to this would be to just catch the exception and print a warning since no actual code needs to be run on RTD.

ChiantiPy needs some tests. I broke the ion class with #51 (fixed by #53), but we should have some tests in place that are run by Travis CI that will alert us before the offending PR gets merged.

Right now Travis just installs and builds the docs (for multiple python versions) so only things like syntax errors get caught. Ideally we would have a few tests that would

• create class instances (e.g. ion, spectrum)
• try out a variety of input arguments to each of these objects
• call some methods
• etc.

The second point is particularly important as there are many ways to instantiate some of these objects and it is easy to change a bit of code without all use cases in mind.

Currently, the ion class does not use ioneq class and instead implements a separate method ioneqOne. Instead, there should be a single object that calculates the ionization equilibrium and any other function/class that needs to access the ionization equilibrium should call (or even subclass) this universal ionization equilibrium class. Since ioneq is already implemented, this is probably relatively low effort.

Astropy provides a fairly straightforward template for building Astropy-like Python packages. SunPy currently uses a version of this template.

They provide bootstrapping for package setup and a lot of helpers for building documentation with Sphinx.

Another big advantage is Astropy's units system as well as the many constants that are builtin.

I think standardizing things like the docs and setup will help to preserve the longevity of ChiantiPy as well as encourage future development of the project. Incorporating it into the Astropy family will also make integration with SunPy easier. This will become important as the solar physics community (hopefully) becomes more Python-centric.

that PR did not build competely

install worked for Python 2.7, 3.4 but not 3.5

none of the tests were built

sphinx-build worked OK for 2.7, 3.4 and 3.5

I run on 3.4.5

I now have an account on Read-the-Docs. When I went to link to to the master copy of ChiantiPy on github, rtd said that there already was a ChiantiPy project there. It looks like it is the example you (wtbarns) built. Would it be OK to remove it?

Currently, the FortranFormat module is bundled with ChiantiPy. I think this was because at some point, the package was not Python 3-compatible and so changes had to be made to the package itself. From what I can tell, it is now compatible with Python 3.

In the interest of slimming down ChiantiPy and not having to manually keep up with any updates in the FortranFormat package, FortranFormat could just be added as a dependency.

The main ChiantiPy webpage appears to be generated by Sphinx. However, the documentation seems to be incomplete. Are there plans to use Sphinx to document the whole API?

Read the Docs seems to be a pretty good option for hosting Python documentation and offers good Sphinx integration. I believe this is what the Sunpy team is using as well.

The unit of the radiative loss rate in the document looks strange to me. I think it should be something like $\Lambda/(n_e n_H)$ where \Lambda is usual volumetric cooling function.
Also, the docstring in freeBoundLoss method says it returns the cooling rate in units of ergs cm^-2 s^-1 which is again conflicting with the y-label on above plot.

There seems to be quite bit of commented out code in ChiantiPy. When version control is not being used, this is of course often necessary to preserve some sort of history. However, now that the codebase has moved to GitHub, I propose deleting these commented out lines as git and GitHub do an excellent job of preserving the history of every line of every file.

This will greatly decrease the total number of lines in ChiantiPy and greatly increase its readability in a few places. I'm more than happy to open a PR to take care of this.

Why does the ioneq object subclass ion? It doesn't seem to use or benefit from any of the methods or attributes of ion

(Sorry @kdere to jump to yet another issue thread! Just want to make sure none of these points/discussions gets buried in other threads)

Continuing discussion from #30,

@wtbarnes

We should think about implementing some automatic builds via Travis CI as well so that it will be easier to say when PRs break the install at least. Later on tests could be added for better coverage. These also get run when a PR is submitted so if a PR breaks the install/fails a test, you know before the merge happens.

@kdere

Sounds like a good idea. I followed the link you gave to 'Travis CI' and logged in via github. Now they want to access my github data. Is that the way to go?

Currently, all of the core parts of the ChiantiPy package are stored in the submodule ChiantiPy.core. However, this extra submodule is really not necessary and creates the need for implicitly importing all modules anytime one is needed, e.g. PR #148

Furthermore, restructuring the package in this way will allow the docs to be structured a bit more sensibly and we can provide more info in the API docs about individual objects/modules (a la SunPy docs)

I propose moving all of the core objects from the ChiantiPy.core dir to the ChiantiPy dir. So importing the ChiantiPy ion object would look like,

from ChiantiPy import ion
# or
import ChiantiPy.ion

without having to import all other modules. It will certainly be faster and I think a bit cleaner.

As @kdere points out in #148, this may create some circular import issues so this may not be as simple as it appears...

We should consider creating a wiki for ChiantiPy. This would be a good place to list more long term goals for the package, e.g. becoming a Sunpy-affiliated package, integrating Astropy units, migrating the old SVN repo. Things like this are not necessarily issues (like bugs or feature requests) and might be better served by being listed as "Long term projects" or something like that.

This can all be done inside the main GitHub page.. By default, the wiki can only be edited by owners and collaborators, but can be made publicly editable. The latter is probably easiest.

I understand that there needs to be both an rst and md version of the README, but it seems that another version (without any extension) has appeared. Is this for PyPI? And if so, why do we have the rst version then?

Can the upsilonDescaleSplups method in the ion class be moved to the tools.archival module? From what I can tell, it is only useful for the old-style .splups files and is replaced by the upsilonDescale method.

How do I test the new docs and dos-strings?

I tried using

sphinx-build -a -b html source build build

after creating the dir 'build' outside of git

it came back that

File "conf.py", line 42, in <module>
ImportError: No module named 'astropy_helpers'

There may be more things missing or a better way to test the docs

The fixes recently introduced in the Continuum module are fairly substantial. We should probably think about doing a release soon so that these changes can propagate to PyPI. It would also be nice to make ChiantiPy available on Anaconda via conda-forge though see discussion in #76

Per the discussion in Issue #134, the method names in the Continuum module need to be cleaned up a bit. Currently, nearly all of them are prepended with calculate (e.g. calculate_free_bound_emission) though this is a bit annoying when searching for a particular method name. To help users identify method names easier, the calculate should be moved to the end of the name. This also helps autocomplete (e.g. in the Jupyter notebook) be more useful. Note that all of the names and calls to these methods in the Continuum module will need to be changed in addition to any other places where a Continuum class is instantiated, e.g. Spectrum, radLoss

Ion.ioneq calculates ionization equilibrium for an entire temperature array. tools.ionrec.ioneq calculates ionization equilibrium at a single temperature. Could these be combined into a single class? Perhaps some of the functions themselves could be combined?

In the continuum class, there are two methods related to the free-bound continuum:

• freeBoundEmiss
• freeBound
  and two methods related to the free-free continuum:
• freeFreeEmiss
  *freeFree

In the free-bound case, according to the documentation, freeBound calculates the rate coefficient (returned in the FreeBound dictionary) while freeBoundEmiss calculates the continuum emissivity. Is the only difference between these two that one (the rate coefficient from freeBound) is multiplied by the abundance and the ionization equilibrium? If so, why can't these just be the same method with a kwarg to include (or not include) these extra quantities?

In the free-free case, it seems that both freeFree and freeFreeEmiss populate the dictionary FreeFree with the same key names, but with different results. Presumably this is the same case as in free-bound: freeFree includes the ionization equilibrium while freeFreeEmiss does not. Either they should populate different dictionaries or they should be combined into a single method with a kwarg to include ionization equilbrium and/or abundance or not.

While the history of the ChiantiPy project goes back to 2009(?), the history of this GitHub repo only goes as far back as last year (2016). The history of the SVN repo on SourceForge should be ported over to this repo so that the history of the first 7 years of the project can be preserved.

See this Stack Overflow answer for a possible method for doing this.

The free-free and free-bound emissivity (calculated using continuum.freeFree() and continuum.freeBound()) does not match the equivalent results in IDL. Relevant plots and code snippets in both IDL and Python are included below.

In both cases, the emission is calculated over 25-414 angstroms in wavelength and 10^5 K to 10^8 K in temperature.

Free-free

Free-bound

Code

Script to produce IDL results,

; set common block
common abund, abund_ref, ioneq, ioneq_logt, ioneq_ref

; read abundance and ionization equilibrium
abundfile="/usr/local/ssw/packages/chianti/dbase/abundance/sun_coronal_1992_feldman.abund"
read_abund, abundfile, abund, abund_ref
ioneqfile="/usr/local/ssw/packages/chianti/dbase/ioneq/chianti.ioneq"
read_ioneq, ioneqfile, ioneq_logt, ioneq, ioneq_ref

; set temperature and wavelength
pressure = 1.e15
temperature = float(ioneq_logt)
wavelength = findgen(413-24) + 25
edensity = pressure/(10.^temperature)

; proton-to-electron density ratio
common proton, pstr, pe_ratio
pe_ratio=proton_dens(temperature)

; calculate free-free
freefree, 10.^temperature, wavelength, ff, /photons, /no_setup
; calculate free-bound
freebound, 10.^temperature, wavelength, fb, /photons, /no_setup

; make into normal units
tp = tp/1d40
ff = ff/1d40
fb = fb/1d40

Script to produce the Python results,

import ChiantiPy.tools.data as ch_data
import ChiantiPy.core as ch
import astropy.constants as const
import astropy.units as u

ch_data.Defaults['abundfile'] = 'sun_coronal_1992_feldman'
ch_data.Defaults['ioneqfile'] = 'chianti'

temperature = 10.**np.arange(5,8,61)
wavelength = np.arange(25,414,1.)
# need to divide by photon energy because ChiantiPy continuum outputs results in 
# erg cm^-3 s^-1 sr^-1 angstrom^-1
photon_energy = ((const.h*const.c).to(u.erg*u.angstrom)/(wavelength*u.angstrom)).value

freefree = np.zeros((len(temperature),len(wavelength)))
freebound = np.zeros((len(temperature),len(wavelength)))

for ion_name in ch_data.MasterList:
    tmp_continuum = ch.continuum(ion_name,temperature=temperature)
    if tmp_continuum.Ion > 1:
        tmp_continuum.freeFree(wavelength)
        if 'rate' in tmp_continuum.FreeFree:
            freefree += tmp_continuum.FreeFree['rate']/photon_energy
        tmp_continuum.freeBound(wavelength)
        if 'rate' in tmp_continuum.FreeBound:
            freebound += tmp_continuum.FreeBound['rate']/photon_energy

I have cleared up the problem with ionization cross-sections but I now need to check the ionization rates. This is the main problem that 0.7.0 is solving and the only outstanding one that I am aware of.

The documentation is improved but certainly not perfect but that can be worked on more after the release.

I want to have a compressed tarball available on github and PyPI. The sourceforge and chiantidatabase sites will then point to these two.

Anything else?

Currently, the Tutorial section in the docs is mostly an out-of-date rehashing of the API documentation and is not needed.

The code examples specific to the ion object could be moved to the ion class description in the API documentation.

I was thinking about making a conda package containing the actual database (as well as one for ChiantiPy) to make installation and distribution easier (also hopefully to fix #67).

However, the http://www.chiantidatabase.org/chianti.html website does not state a license just that

The CHIANTI package is freely available.
If you use the package, we only ask you to appropriately acknowledge CHIANTI .

which is not a licence, and would be considered by many to be a non-free license because of the apparent requirement to cite CHIANTI. The database should be under some more formal license so that we know our position with respect to re-distributing the data.

ping @dpshelio who might have a better idea about what licenses would be appropriate for data?

For the last day or so, all of our Read the Docs builds have failed because of a timeout at ~900 s. Typically, the entire build should only take about ~400 s.

Each of the build steps seems to be taking ~10x as long as it should compared to past successful builds. See build logs here.

I don't believe any part of the docs has changed in the last few commits so its possible this is due to an upstream change in one of the packages we install on RTD.