BradGreig / Hybrid21CM

1 stars 3 forks source link

OUTPUT_AVE is redundant? #27

Closed steven-murray closed 5 years ago

steven-murray commented 5 years ago

I'm thinking having the OUTPUT_AVE option is a bit redundant and a bit confusing. At the moment, all it seems to do is print out information about global averages, which is not really "outputting" as I would understand it. These values should be saved to the output structures themselves, if they are important to keep track of.

In fact, for values such as global_xH, this can be easily added to the python-side object as a simple mean over the xH box which it already has. I'm wondering if the other average quantities can be gotten similarly. Eg:

if(flag_options->OUTPUT_AVE) {
                            J_alpha_ave += J_alpha_tot;
                            xalpha_ave += xa_tilde_fast;
                            Xheat_ave += ( dxheat_dzp );
                            Xion_ave += ( dt_dzp*dxion_source_dt_box[box_ct] );
                            Ts_ave += TS_fast;
                            Tk_ave += T;
}

I know at least Ts and Tk are output as boxes as part of the TsBox struct. Their average should be able to be calculated at will by the user from the python side. I don't know about the other quantities here. If they can't be directly calculated from already-output boxes, we should consider adding their average as a float quantity in the TsBox struct, and adding them to that. If you think this should still be optional, we could have an option like "COMPUTE_AVERAGES" which if set to True would calculate these averages and add them to the struct, and otherwise the struct would just have a random value (and python would know about this and just raise an AttributeError if you try to access it).

Just want to make sure this makes sense, @BradGreig ?

BradGreig commented 5 years ago

For the most part, I have been using OUTPUT_AVE as a debugging tool. And this is essentially all I really see it as. By outputting the globally averaged components of the Ts function (and ionise etc.) I can verify that the algorithm is all in working order (i.e. no errors or anything).

Already, xH and Tb should be kept as this is useful data, the remainder is less interesting or useful.

In that sense, you can switch OUTPUT_AVE to some form of existing debugging.

steven-murray commented 5 years ago

Ok excellent. I thought this might be the case. In the current version I've already switched the printing of these values to INFO level logging statements. I guess since they really are debug tools I could change that to DEBUG level. This wouldn't print out by default j less the user recompiled the package to DEBUG. I'll make sure that the global xH and Tb are part of the python side objects.

On Thu, Feb 7, 2019, 4:43 PM BradGreig <notifications@github.com wrote:

For the most part, I have been using OUTPUT_AVE as a debugging tool. And this is essentially all I really see it as. By outputting the globally averaged components of the Ts function (and ionise etc.) I can verify that the algorithm is all in working order (i.e. no errors or anything).

Already, xH and Tb should be kept as this is useful data, the remainder is less interesting or useful.

In that sense, you can switch OUTPUT_AVE to some form of existing debugging.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/BradGreig/Hybrid21CM/issues/27#issuecomment-461637783, or mute the thread https://github.com/notifications/unsubscribe-auth/ABNo3rrimvu-xNfM0N3erTloHCxaIqeWks5vLLnygaJpZM4ap1r8 .

steven-murray commented 5 years ago

I've done this, and removed the OUTPUT_AVE flag from FlagOptions in ee87479a93f25e095f35e8aa05fd781b38ca34fe. To print out this info, you just need to install the package with LOG_LEVEL=DEBUG pip install . The main global properties are also available in the Python structures as <>.global_xH etc. They are lazily evaluated then cached in memory.