sirocco-rt / sirocco

This is the repository for Sirocco, the radiative transfer code used to model winds in AGN and other systems
GNU General Public License v3.0
30 stars 24 forks source link

Some pull requests are failing the balmer tests #1055

Closed kslong closed 9 months ago

kslong commented 9 months ago

When one attempts to merge into dev some pull requests are failing the Balmer test. We need to understand why this is, and we need to have a procedure that checks any tests that will be performed on github earlier in the process.

Note that it is not immediately clear why the Balmer test is failing though I have confirmed that the Polynesian branch does frail the test locally, and that the error is significant

The output from py87h, which passes says:


Array of line ratio relative errors for Balmer series:
 [0.0251518  0.         0.04412924 0.06414727 0.11292285 0.1057267
 0.11958364 0.13849552]

Test passed?: True.

The output from py87i, which fails says:

Array of line ratio relative errors for Balmer series:
 [0.02159244 0.         0.08995894 0.18186688 0.31303101 0.36087883
 0.41898865 0.48026316]

Test passed?: False

Note that the test compares calculated line ratios o the numbers in Osterbrock, so a value of 0.48 means, one deviates by 50% (in one direction or another from Osterbrock.)

The tests is currently set to fail if any of the values printed above exceed a TOLERANCE of 0.3

kslong commented 9 months ago

OK, I understand why py87i is failing the Balmer tests as it currently exists. The balmer_test.pf file uses ML93 ionization prescription, where I had made a change. Previously, in dev, the ML93 prescription solved for T_e in the normal way, which I think was a mistake that had crept in. In ML93, see the end of section 2.2, they set T_e = 0.9 T_r. I discovered we were not doing this for the Tardis comparisons, and this is one reason we were getting poorer Tardis comparisons than previously. So in py87i, I had changed this.

If one changes the ionization prescription to matrix_bb. Then the results of the Balmer test for 87i are as follows:

Array of line ratio relative errors for Balmer series:
 [0.01894222 0.         0.04730428 0.06722004 0.11131465 0.11605118
 0.12662894 0.14755854]

Test passed?: True

which is very similar to what was being obtained with py87h (dev)

Do you @jhmatthews see any reason not to change the Balmer test to use matrix_bb (which will separate the issue of the Balmer test from any mods to ML93 going forward.

kslong commented 9 months ago

I have verified that with the suggested change the Polynesia branch passes all of the github checks.

kslong commented 9 months ago

Polynesia branch has been merged into dev.