Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added new fitacf 3 fields #39

Merged
merged 4 commits into from Sep 21, 2022
Merged

added new fitacf 3 fields #39

merged 4 commits into from Sep 21, 2022

Conversation

mts299
Copy link
Collaborator

@mts299 mts299 commented Feb 3, 2022

Scope

@ecbland pointed out new fields will be added to the fitacf 3 file format thus here is the corresponding pyDARNio change for reading the fitacf files

issue: #38

Approval

Number of approvals: 1-2

Test

should be the normal read_fitacf() command we use with the a test file from #38 provided by @ecbland

@mts299 mts299 added the enhancement New feature or request label Feb 3, 2022
@mts299 mts299 self-assigned this Feb 3, 2022
@mts299 mts299 linked an issue Feb 3, 2022 that may be closed by this pull request
6 tasks
@ecbland
Copy link
Collaborator

ecbland commented Feb 3, 2022

@mts299 There's a test fitacf file on the issue I opened yesterday.

@mts299
Copy link
Collaborator Author

mts299 commented Feb 3, 2022

My bad :p trying to avoid staring at a screen ;)

@ecbland
Copy link
Collaborator

ecbland commented Feb 4, 2022

@mts299 In commit 81e1edc I've separated the field names into 3 separate groups:

  • xcf_fields: these fields are common to all fitacf files containing xcf data
  • xcf_fields_fitacf3: the fields that are unique to fitacf3 files (elv_fitted, elv_error)
  • xcf_fields_fitacf2: the fields that are unique to fitacf2 files (x_qflg, x_p_l,...,elv_low, elv_high,...)

I tested this with fitacf2 and fitacf3 files created using the current file format, and also with the new format (fitacf3 only). Here are the 3 test files: test_files.zip

Is this sensible?

@carleyjmartin
Copy link
Collaborator

@RemingtonRohel just FYI: the DVWG has their eye on this PR, we're hoping to merge to develop after testing soon and do a new release before RST is released to have this available before the new fitacf3 are able to be made in RST. Is the borealis restructure PR something you're interested in getting into a new release?

Depending on how long it takes for the other two fitacf changes going to happen to make its way through the dev and rubber stamping phases of RST, we will try to get them in the same release too.

Copy link
Collaborator

@carleyjmartin carleyjmartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, tested with the following code on the data files provided by @ecbland :

import pydarnio

fitacf2_file = "/Users/carley/Desktop/20180323.0801.00.lyr.fitacf2"
fitacf3_file = "/Users/carley/Desktop/20180323.0801.00.lyr.fitacf3"
newfitacf3_file = "/Users/carley/Desktop/modified_format.lyr.fitacf3"

fitacf2_read = pydarnio.SDarnRead(fitacf2_file)
fitacf3_read = pydarnio.SDarnRead(fitacf3_file)
newfitacf3_read = pydarnio.SDarnRead(newfitacf3_file)

fitacf2_data = fitacf2_read.read_fitacf()
fitacf3_data = fitacf3_read.read_fitacf()
newfitacf3_data = newfitacf3_read.read_fitacf()

print('Fitacf 2')
print(fitacf2_data[0])
print('Fitacf 3')
print(fitacf3_data[0])
print('New format Fitacf 3')
print(newfitacf3_data[0])


# Just to test:
# MAKE SURE THAT INSTALLING PYDARN AFTER DOES NOT REVERT PYDARNIO TO THE 
# RELEASE VERSION OF 1.1 INSTEAD OF THE APPLICABLE BRANCH
import pydarn
import matplotlib.pyplot as plt

pydarn.RTP.plot_summary(fitacf2_data, beam_num=7)
plt.show()

pydarn.RTP.plot_summary(fitacf3_data, beam_num=7)
plt.show()

pydarn.RTP.plot_summary(newfitacf3_data, beam_num=7)
plt.show()

The printed data has the corresponding fields for each version of fitacf. I just plotted them out too to check and here are the results (looking good from pydarn's end too):

fitacf2:
Screen Shot 2022-09-13 at 11 09 08 AM

fitacf3:
Screen Shot 2022-09-13 at 11 09 21 AM

new fitacf3:
Screen Shot 2022-09-13 at 11 09 35 AM

@RemingtonRohel
Copy link
Collaborator

@carleyjmartin Sorry for taking so long to get to this! I'm taking another look at the restructure PR today, I would love to roll these both into a new release if we can! Since there isn't much else going on with this package I think bundling them is a good idea, so I'll try to be speedy.

@Shirling-VT
Copy link
Collaborator

@ecbland I tested using the codes Carley provided above, I can print and see (x_qflg, x_p_l,...,elv_low, elv_high,...) fields in both the fitacf2 and fitacf3 files without modified format. When I print data in the fitacf3 with modified format, I can only see elv_fitted and elv_error fields but not the elv_low and elv_high fields. Are these expected output? If so, I think we can merge this PR. But I may misunderstand how you separate fields in the three files, my output does not seem to fully agree with your description below:
I've separated the field names into 3 separate groups:
xcf_fields: these fields are common to all fitacf files containing xcf data
xcf_fields_fitacf3: the fields that are unique to fitacf3 files (elv_fitted, elv_error)
xcf_fields_fitacf2: the fields that are unique to fitacf2 files (x_qflg, x_p_l,...,elv_low, elv_high,...)

What do you think?

@ecbland
Copy link
Collaborator

ecbland commented Sep 21, 2022

@Shirling-VT Thanks for testing. It sounds like you're getting the expected behaviour. The "separating field names into 3 groups" (81e1edc) is purely to prevent the "missing field" errors. If you were able to read all of the sample files then this is ready to merge.

@Shirling-VT Shirling-VT merged commit 2867be8 into develop Sep 21, 2022
@carleyjmartin carleyjmartin deleted the ehn/new_fitacf3_fields branch September 28, 2022 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updates to elevation angle fields for FitACF 3.0
5 participants