cloudy-astrophysics / bug-tracker-migration-test

Trial run for importing the nublado.org Trac tickets as GitHub issues
0 stars 0 forks source link

A significant float/double issue in blr_fp89.in (trac #84) #87

Closed cloudy-bot closed 5 years ago

cloudy-bot commented 15 years ago

reported by: rporter

I just uncovered a fairly major issue on the trunk at r2757 that appears to have been introduced on the newsolvers branch. In r2538 you modified a number of input scripts to reflect new results on that branch (just before merging back onto the trunk in r2539). Most of the models only had a few changes. But blr_fp89.in had 15. Hbeta dropped by 20%. “Pa C” dropped nearly in half. “Fe2h” by nearly a factor of 3. Turns out most of those changes only happened in the default mixed-precision case. The double-precision case still basically gets the same answers it did before the newsolvers merge. I wonder if that might shed some light on the platform-dependent issues that had previously popped up.

Migrated from https://www.nublado.org/ticket/84

{
    "status": "closed",
    "changetime": "2009-02-23T14:55:51Z",
    "_ts": "1235400951000000",
    "description": "I just uncovered a fairly major issue on the trunk at r2757 that appears to have been introduced on the newsolvers branch.  In r2538 you modified a number of input scripts to reflect new results on that branch (just before merging back onto the trunk in r2539).  Most of the models only had a few changes.  But blr_fp89.in had 15.  Hbeta dropped by 20%.  \u201cPa C\u201d dropped nearly in half.  \u201cFe2h\u201d by nearly a factor of 3.  Turns out most of those changes only happened in the default mixed-precision case.  The double-precision case still basically gets the same answers it did before the newsolvers merge.  I wonder if that might shed some light on the platform-dependent issues that had previously popped up.\n",
    "reporter": "rporter",
    "cc": "",
    "resolution": "fixed",
    "time": "2009-02-22T13:34:50Z",
    "component": "radiative transfer",
    "summary": "A significant float/double issue in blr_fp89.in",
    "priority": "major",
    "keywords": "ots damper",
    "version": "",
    "milestone": "",
    "owner": "peter",
    "type": "defect - convergence"
}
cloudy-bot commented 15 years ago

@peter-van-hoof-noaccount commented:

Gary commented:

The entry " Fe2h 0" is bogus and should be removed. It is a net residual heating term which is the difference between two large heating and cooling terms. It is not an observed property. I removed this from many of the other sims and recommend removing it from this one.

cloudy-bot commented 15 years ago

@peter-van-hoof-noaccount commented:

I think I finally got to the bottom of this. The first thing I noticed was that the mixed and double results agreed very well on the first iteration, which makes temperature solver issues quite unlikely. In the second iteration results start to diverge considerably, and you can see that in the ionization structure. In the mixed precision model e.g. carbon recombines much earlier than in the double precision model. When I checked the ionization rates, it became clear that this was caused by direct photoionization, and even closer inspection revealed that the otslin rate in cell 2206 differed by nearly 60 orders of magnitude between the two codes (2.8e-45 vs. 2.1e+15)! Surely this must be some sort of record!!

It looks like this is a bad case of Evil OTS Dampers. The culprit seems to be the code on lines 589:614 of [browser:trunk/source/rt_ots.cpp@2757#L589 rt_ots.cpp]. If you replace that with a straight copy of otslinNew to otslin (and similar for otscon), the mixed and double codes agree quite well (and completely disagree with what is on the trunk now).

cloudy-bot commented 15 years ago

@peter-van-hoof-noaccount commented:

The line at cell 2206 of otslin appears to be the FeII 1080 transition. Gary and Ryan spent some time working with this transition - there was a problem with a strong maser for some sims. The maser may have been a result of ots oscillations or their damping. To work around this problem, hacks were introduced in r1472 and r1795. After reverting these changes (i.e. removing all the "( iteration == 1 )" clauses), the code still seems to be running OK and reports (nearly) the same results.

cloudy-bot commented 15 years ago

This problem has been resolved on the trunk in r2758.

Botched asserts were fixed in r2759.

This fix is too invasive to be fixed on c08_branch, so closing this as a wont-fix on c08_branch.