innosat-mats / MATS-L1-processing

Python code for calibrating MATS images
MIT License
0 stars 1 forks source link

temperatuere set to average of uv1 and uv2 in read_parquet #212

Closed lindamegner closed 7 months ago

lindamegner commented 8 months ago

Changed to that the tempreature of all CCDs are set to the average between the ones measured close to UV1 and UV2. This was not compliant with other parts of the code where this was updated and pushed about a month ago. I am not sure where this _read_parquet_funtion is used.

Please enter the commit message for your changes. Lines starting

lindamegner commented 7 months ago

OK, if it is only used in older versions of the data. Then there is no need to change it I assume? Since in those versions we only used to set the temperatures to what it was in UV1, not the average. In fact could it be bad to change it ? It may be more consistent to keep it like it is. What do you reckon?

skymandr commented 7 months ago

OK, if it is only used in older versions of the data. Then there is no need to change it I assume? Since in those versions we only used to set the temperatures to what it was in UV1, not the average. In fact could it be bad to change it ? It may be more consistent to keep it like it is. What do you reckon?

I think that @donal-mur and @OleMartinChristensen can answer that question better than I can.

OleMartinChristensen commented 7 months ago

Should this be merged in or dropped @lindamegner? I think we should not change the behaviour of old data versions (even if it is during the read in process). Its better to just use the v0.7 data if you want these temperatures, optionally add a conversion function in mats-utils

lindamegner commented 7 months ago

I agree.

lindamegner commented 4 months ago

Hmm. I looked at the code now and realise we still set CCDitem["temperature"] = HTR8A. I.e. the used temperature is only based the measurement on UV2. It probably does not make a big difference but we do not seem to use the average as decided. At a quick look at the code suggest we dont.

skymandr commented 4 months ago

Hmm. I looked at the code now and realise we still set CCDitem["temperature"] = HTR8A. I.e. the used temperature is only based the measurement on UV2. It probably does not make a big difference but we do not seem to use the average as decided. At a quick look at the code suggest we dont.

This is supposed calculated in L1a, not L1b. This is what is done there: https://github.com/innosat-mats/level1a/blob/main/level1a/handlers/mats_utils/ccd_item.py

Specifically this aims to implement what is done in get_temperature.py in this repo: https://github.com/innosat-mats/MATS-L1-processing/blob/08dc7119f563c5ef385679a6e87e0df6b1d2b68e/src/mats_l1_processing/get_temperature.py#L38C3-L94C19

Is this wrong, @lindamegner?

lindamegner commented 3 months ago

Well, your replica is correct but the original is not good (ie my fault). I do not see why we have two different temperatures temperature_HTR and temperature They should both be = 0.5 * (ccd_data["HTR8A"] + ccd_data["HTR8B"])

I think we should remove one of them the one called temperature_HTR and only have ccd_data["temperature"] = 0.5 * (ccd_data["HTR8A"] + ccd_data["HTR8B"])

temperature_HTR does not seem to be used anyway

@OleMartinChristensen You dont see any reason for having different values? I think we had some idea that temperature could be set to one of the measured temperatures varibales but then we seem to have added temperature_HTR too for some reason that at least I dont remember. Do you?

skymandr commented 3 months ago

In other words, the data in L1A and/or L1B is wrong?

lindamegner commented 2 months ago

Well, from what I can see ccd_data["temperature"] is the only one used in the L1b calibration and is set to ccd_data["temperature"] = ccd_data["HTR8A"] in level 1a, instead of 0.5 * (ccd_data["HTR8A"] + ccd_data["HTR8B"]). So yes, technically both L1a and L1b are wrong in the sense that it was not what we meant to do. But the effect should be tiny and not noticable in the data. I dont know if it is worth rerunning L1a (what do you reckon @OleMartinChristensen ) but we should remove ccd_data["temperature_HRT"] since it is not used. And set ccd_data["temperature"] = 0.5 * (ccd_data["HTR8A"] + ccd_data["HTR8B"]).