Opened 6 years ago

Closed 3 years ago

#204 closed defect - wrong answer (fixed)

improve frequency mesh and ipoint

Reported by: peter Owned by: nobody
Priority: major Milestone: C13 branch
Component: infrastructure Version: trunk
Keywords: Cc:

Description

The first problem is that the code in many places implicitly assumes that the frequency cell extends from anu[i]-widflx[i]/2 to anu[i]+widflx[i]/2. That is not correct, it extends from (anu[i-1]+anu[i])/2. to (anu[i]+anu[i+1])/2, which is not the same thing... You could argue that having it extend from sqrt(anu[i-1]*anu[i]) to sqrt(anu[i]*anu[i+1]) would be a better choice, but we should discuss that further. What we need are arrays that give the lower and upper bound of the cell. These are currently in grainvar (gv.anumin and gv.anumax), but should be moved to rfield and then used consistently throughout the code. Strictly speaking it is not necessary to have two arrays since they will be largely redundant. But for code clarity I think it is better to do it this way, and the overhead should be minimal.

Second, I am not convinced that ipoint will always return the correct pointer. This looks like very convoluted code that needs to be replaced by a bisection hunt. Not to mention the fact that it needs to be put on the C scale... So I propose that we create inline functions ipointC() and ipointF() that return pointers determined by bisection on the C- and Fortran-scale, resp. The latter should then be phased out and the code gradually moved over to ipointC(). Using Hungarian notation to discriminate between pointers on the C and Fortran scale could be a useful tool in this process.

Change History (2)

comment:1 Changed 6 years ago by Gary J. Ferland

I don't understand the motivation for a bisection hunt. That will certainly cost cycles.

Why does the equation in section S2.6.2 of hazy2_10.pdf pointed out by Robin fail? There are asserts at cont_ipoint.cpp:78 & 79 to insure that the cell number is valid. Do you suspect cases where the cell number is invalid but it still gets past the asserts?

comment:2 Changed 3 years ago by peter

Resolution: fixed
Status: newclosed

The frequency mesh problems have been fixed on the newmesh branch. This was merged to the trunk in r10281.

Note: See TracTickets for help on using tickets.