ACHMartin / seastar_project

4 stars 0 forks source link

Update to fill_missing_variables #224

Closed DavidMcCann-NOC closed 1 year ago

DavidMcCann-NOC commented 1 year ago

Here's my attempt at making sure that the mid antenna variables are properly filled with NaNs to address issue #221

DavidMcCann-NOC commented 1 year ago

The modification you added looks great. Do we still need the hold portion of code which is faulty? Did you try to solve the -999 issue?

I've kept the other two bits of merging code to remain for the moment as they pull in CalImage from the Mid dataset into the other two - without that they merge_beams code will still crash. Alternatively we can drop the CalImage variable upstream

DavidMcCann-NOC commented 1 year ago

I did try and solve the -9999 issue, I have a piece of code that works nicely but strangely it gets stuck on a variable called Dummy that is actually the identifier for those -9999 values. In this case we have a 1x3 array of -9999 (along the antenna dimension), and when I try and replace the numbers with NaN using the same code that works for all the other arrays it complains about turning a NaN into an int.... I'll have it sorted soon i think

ACHMartin commented 1 year ago

I did try and solve the -9999 issue, I have a piece of code that works nicely but strangely it gets stuck on a variable called Dummy that is actually the identifier for those -9999 values. In this case we have a 1x3 array of -9999 (along the antenna dimension), and when I try and replace the numbers with NaN using the same code that works for all the other arrays it complains about turning a NaN into an int.... I'll have it sorted soon i think

This sounds a very good surprise. So if I understand correctly, you only need to change this field to NaN to remove all the -999? That would be handy

ACHMartin commented 1 year ago

The modification you added looks great. Do we still need the hold portion of code which is faulty? Did you try to solve the -999 issue?

I've kept the other two bits of merging code to remain for the moment as they pull in CalImage from the Mid dataset into the other two - without that they merge_beams code will still crash. Alternatively we can drop the CalImage variable upstream

What do we have in this field? Is it constant? Does it change with Tracks?

DavidMcCann-NOC commented 1 year ago

The modification you added looks great. Do we still need the hold portion of code which is faulty? Did you try to solve the -999 issue?

I've kept the other two bits of merging code to remain for the moment as they pull in CalImage from the Mid dataset into the other two - without that they merge_beams code will still crash. Alternatively we can drop the CalImage variable upstream

What do we have in this field? Is it constant? Does it change with Tracks?

Its the calibration constants I think - I remember Guiseppe asking about it in that meeting a while ago. I haven't looked at it fully yet

DavidMcCann-NOC commented 1 year ago

I did try and solve the -9999 issue, I have a piece of code that works nicely but strangely it gets stuck on a variable called Dummy that is actually the identifier for those -9999 values. In this case we have a 1x3 array of -9999 (along the antenna dimension), and when I try and replace the numbers with NaN using the same code that works for all the other arrays it complains about turning a NaN into an int.... I'll have it sorted soon i think

This sounds a very good surprise. So if I understand correctly, you only need to change this field to NaN to remove all the -999? That would be handy

Indeed it would! Unfortunately I think its literally just an integer that serves as a reminder of what the null value in the datasets is. So we can either use it to set the -9999 (i.e. just refering to ds.Dummy), or we can ignore it and hard-cose the number in.

ACHMartin commented 1 year ago

I did try and solve the -9999 issue, I have a piece of code that works nicely but strangely it gets stuck on a variable called Dummy that is actually the identifier for those -9999 values. In this case we have a 1x3 array of -9999 (along the antenna dimension), and when I try and replace the numbers with NaN using the same code that works for all the other arrays it complains about turning a NaN into an int.... I'll have it sorted soon i think

This sounds a very good surprise. So if I understand correctly, you only need to change this field to NaN to remove all the -999? That would be handy

Indeed it would! Unfortunately I think its literally just an integer that serves as a reminder of what the null value in the datasets is. So we can either use it to set the -9999 (i.e. just refering to ds.Dummy), or we can ignore it and hard-cose the number in.

What I understand: ds.Dummy keeps the reference about what a BAD value is, but if we change this value in ds.Dummy it will not change all the BAD values in ds. If the things above is correct, we could do something like:

nan_flag = np.unique(ds.Dummy.data)
TODO replace all nan_flag with np.nan

so no need to hardly put the -999 in the code.

DavidMcCann-NOC commented 1 year ago

Added the working function to replace the dummy values

DavidMcCann-NOC commented 1 year ago

I would like something between 4. and 1.

Today in the processing, what I copied from you was:

1 for track in star_pattern_tracks.keys(): # Loop through star pattern tracks 2 file_index = star_pattern_tracks[track] 3 ds = ss.utils.readers.load_OSCAR_data(oscar_path, file_time_triplets[file_index][1]) 4 antenna_ident = ss.utils.tools.identify_antenna_location_from_filename(oscar_path, file_time_triplets[file_index][1]) 5 ds = ss.oscar.level1.fill_missing_variables(ds, antenna_ident) 6 print('Processing file index',file_index) 7 # L1 processing 8 for i in list(ds.keys()): 9 ds[i] = ss.oscar.level1.check_antenna_polarization(ds[i]) 10 ds[i] = ss.oscar.level1.compute_multilooking_Master_Slave(ds[i], window=7) 11# ds[i]['Baseline'] = ss.oscar.level1.compute_antenna_baseline(0.2) 12 ds[i] = ss.oscar.level1.compute_antenna_azimuth_direction(ds[i], antenna=antenna_ident[list(ds.keys()).index(i)]) 13 ds[i] = ss.oscar.level1.compute_time_lag_Master_Slave(ds[i], options='from_SAR_time') 14 ds[i] = ss.oscar.level1.compute_radial_surface_velocity(ds[i]) 15 #Build L1 dataset 16 dsl1[track] = ss.oscar.level1.merge_beams(ds, antenna_ident)]

I think line 5 can be used later, probably only just before line 16 as we only use one antenna at a time. Line 10: it will be better if we only get the variable we wants and drop all the Level1-A data (SLC, ...). The output will be a new >dataset. All the line 9, 12-14 can work on this new dataset. Just before merging, we fill in the missing fields (e.g. interferogram, ...) and we merge.

What do you think about this approach?

We can chat about this in person - I think I understand and agree - shouldn't require many more changes

DavidMcCann-NOC commented 1 year ago

Added a commit to address what we chatted about on Friday. This works to L1b level now (i think), and is a little more intuitive than it was previously. It also no longer uses up all of my memory which is good.