Opened 3 years ago

Closed 5 months ago

#310 closed defect - wrong answer (fixed)

emergent line intensities are incorrect

Reported by: peter Owned by: nobody
Priority: blocker Milestone: C17_branch
Component: radiative transfer Version: trunk
Keywords: Cc:

Description

In C13.03 there are several problems with the emergent luminosities and line ratios predicted by the code. So far two issues have been found.

1 - When optimizing emergent line ratios, the intrinsic luminosity of the normalization line is used rather than the emergent luminosity.

2 - For some lines the intrinsic and emergent luminosity are identical.

Bug reported by Jim Smith.

Change History (9)

comment:1 Changed 3 years ago by peter

Issue 1 listed above was fixed in r9728 on the trunk and r9729 on c13_branch.

comment:2 Changed 3 years ago by peter

Issue 2 listed above was traced to the use of linadd() vs lindst(). Both routines call lincom(), where a call coming from linadd() activates this code

if (wavelength > 0 )
{
    /* no need to increment or set [1] version since this is called with no continuum
     * index, don't know what to do */
    /* only put informational lines, like "Q(H) 4861", in this stack
     * many continua have a wavelength of zero and are proper intensities,
     * it would be wrong to predict their transferred intensity */
    LineSv[LineSave.nsum].emslin[1] = LineSv[LineSave.nsum].emslin[0];
    LineSv[LineSave.nsum].SumLine[1] = LineSv[LineSave.nsum].SumLine[0];
}

This code still exists on the trunk. Calls to linadd() are still done as well, but may now respect the comment above. I did not check that very carefully. This is worth double-checking. However, I think there are still remaining problems, more on that later.

I think that the root cause of this problem is something that I have been complaining about for some time, namely that the line stack is a jumble of things that simply do not belong together. In my opinion the line stack should consist solely of objects that have a wavelength, a local emissivity and a local opacity at any location in the cloud, so that you can do consistent RT on these objects. Anything that doesn't fit in that definition should be removed from the stack. We can add them elsewhere in the output, that is fine. But they have no business being in the line stack.

This is clearly too big a target for C15, but I think that we should be working towards cleaning up the line list and making sure that it becomes a uniform body of objects.

Now back to C15. I see at least one problem spot in the current code. PutLine?() calls linadd() to add the so-called "informative" entries like the pumping contribution. The fact that they don't go through the usual emergent line RT means that they are at best useless, and at the worst completely misleading as they cannot be compared to the emergent intensity of the main line. However, that would be the main point for adding them...

Another question is how we handle blends? I have not looked up that code, but we should check whether these could be suffering from the same problem. Maybe there are other problem spots?

comment:3 Changed 3 years ago by peter

I have looked at the code and found that the Blnd lines are added with linadd() on the trunk, meaning that the blends will be inconsistent with the sum of the component lines. The lines Jim was complaining about were also blends, so my bet is that this bug is still alive and kicking on the trunk...

My vague recollection is that the distinction linadd / lindst had to do with not double-counting some contribution from the emission line. Probably something like not getting it added twice in the save continuum output. So the short term solution may be to add even more complexity and create another routine for adding lines where they do get the proper RT treatment, but are not added in the spectrum (assuming that is the only side-effect we need to worry about). That means more "just get it 'working'" magic...

Anything more fundamental seems hopeless for C15.

comment:4 Changed 3 years ago by peter

I think that for all "lines" added with linadd(), the emergent entry should be zeroed rather than copied from the intrinsic entry, i.e. the code shown above should be modified to

if (wavelength > 0 )
{
    /* no need to increment or set [1] version since this is called with no continuum
     * index, don't know what to do */
    /* only put informational lines, like "Q(H) 4861", in this stack
     * many continua have a wavelength of zero and are proper intensities,
     * it would be wrong to predict their transferred intensity */
    LineSv[LineSave.nsum].emslin[1] = 0.;
    LineSv[LineSave.nsum].SumLine[1] = 0.;

This avoids the user being tricked into invalid comparisons between intrinsic and emergent quantities. Entries with zero wavelength should probably also be zeroed on the emergent line stack.

comment:5 in reply to:  3 Changed 3 years ago by rjrw

Replying to peter:

I have looked at the code and found that the Blnd lines are added with linadd() on the trunk, meaning that the blends will be inconsistent with the sum of the component lines. The lines Jim was complaining about were also blends, so my bet is that this bug is still alive and kicking on the trunk...

On the trunk, calls to emslinSet(), etc., are ignored for blends, and blend properties are calculated on the fly by totalling the contributing components (see lines.h).

comment:6 Changed 3 years ago by rjrw

Emergent intensities appear to get 'stuck' when transitions are trimmed...

comment:7 Changed 3 years ago by rjrw

The fix to this last problem is (as one might expect) a few lines below the place where #309 fixed the intrinsic intensities.

Fixed at r9971 on the trunk, will also post to c13_branch.

comment:8 Changed 5 months ago by Gary J. Ferland

We have done extensive testing, showing emergenet / intrinsic vs wavelength for dusty models, and results now appear to be correct. I believe that this is mostly fixed. During the fix the inward part of emergent fell off the table and needs to be reported, but that is another, simpler, ticket. I am closing this ticket.

comment:9 Changed 5 months ago by Gary J. Ferland

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.