Inconsistent default behavior re: line lists

[clairekope]

Small issue I wanted to document while testing something else. It appears trident.make_simple_ray and SpectrumGenerator.make_spectrum have different default behavior when no line list is provided. The former assumes no lines, and the latter assumes all lines in the default database. Not a realistic use case, but does cause failures because (for Enzo) the metallicity field isn't added to the ray object even though H_nuclei_density is:

ray = trident.make_simple_ray(ds, [0,0,0], [1,1,1])
sg = trident.SpectrumGenerator('COS-G130M')
sg.make_spectrum(ray)

Briefly, results in

yt : [INFO     ] 2021-05-26 13:36:56,591 Creating H_number_density from ray's density, temperature, metallicity.
yt : [INFO     ] 2021-05-26 13:36:56,612 Creating C_number_density from ray's density, temperature, metallicity.
YTFieldNotFound: Could not find field '('all', 'C_number_density')' in ray.h5.

During handling of the above exception, another exception occurred:

YTFieldNotFound: Could not find field '('gas', 'metallicity')' in ray.h5.

This might also apply to the other types of ray generators.

[chummels]

Thanks for the message. Yeah, I guess I assumed that people would have a metallicity field if there were actually plotting a spectrum, but I suppose it's possible that they just want to include hydrogen lines. I'm not exactly sure what to put as the default for the lines kwarg in make_spectrum(). Clearly the only way to prevent errors for missing fields is to set it to None by default, but that seems pretty dumb to have the default make a featureless spectrum. I guess we could make lines a required argument. What do you think would be a good way of resolving this problem in terms of your intuition for what is expected here?

[clairekope]

I think the default for make_spectrum() makes sense. I would suggest having make_simple_ray save the metallicity field regardless, but that might make trouble for simulation types I'm less familiar with? Or it could be, "save the metallicity field regardless (if the dataset has one)"

[chummels]

Well, the issue is that Trident builds ion fields differently depending on what fields are present in the dataset. If someone has a Mg_metallicity field, Trident will use that when constructing Mg_ion_number_density fields rather than assuming solar abundances from the metallicity field. And I'm a bit hesitant to just pull in all possible metallicity fields into a ray that may be present on the dataset.

[clairekope]

I agree, adding all possible metallicity fields isn't a great solution. I would say, either make_simple_ray et al should require a line list, or the default can just be left as is, because it isn't a realistic application. I only ran into while making a minimally function test case.

[clairekope]

After our discussion I don't really think this needs changing

[chummels]

I've been thinking of a way to better do this and account for your expectations. I am going to be tweaking the way in which the line lists work in the near future, and I'll try to keep your suggestions in mind. I may request your review on a PR when I do. Thanks, @clairekope !