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

Update aircraft pairing (no changes to results) #305

Merged
merged 2 commits into from
Nov 12, 2024

Conversation

rschwant
Copy link
Collaborator

The ASIA-AQ WRF-Chem test NaNed many points during the pairing step. It looks like because there were tacked on 0.000001 values to the end of some of the numbers in lat, lon, and pressure (e.g., 120.0000001) and then the vertical pairing step could not match these values properly. I added code to round before this pairing occurs to truncate these extra digits. @colin-harkins and @zmoon, is there a better way to do this? I've seen this happen before with pulling in .txt files into Python and I'm not sure why sometimes this happens. Is this rounding approach an okay fix? I want to make sure this code works for all icartt files even if there are some odd formatting, so I think this is safer.

I also as I was going through the code to find the problem realized that we really only want to replace the pressure_model NaNed points with pressure_obs when the pressure_obs is outside of the range of the model data. Right now it is doing this for all time steps. I tested that you get the same results doing both methods because merge_asof "nearest" approach selects the right point anyway in FIREX-AQ, RECAP, and AEROMMA, but I think the code is cleaner this way and less likely to have problems later on.

Let me know what you all think! Thanks!

Copy link
Collaborator

@zmoon zmoon left a comment

Choose a reason for hiding this comment

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

I don't think a vertical interp routine should modify coordinates in place. In principle, such correction should happen in the reader.

@rschwant
Copy link
Collaborator Author

We could try correcting this after the icartt files are read in. And see if this also works to fix the problem too. I can look into this next week as well.

@colin-harkins
Copy link
Collaborator

I don't see any issues with your fix for the fillna code for the pressure model and agreed we shouldn't be replacing the values when pressure_obs is outside of the range of the model data. Maybe we can separate this from the fix for the extra digits in the lat/lon coordinates so that we can get this fix in and leave some time to figure out how to fix the coordinates in the reader.

@rschwant
Copy link
Collaborator Author

@colin-harkins, I updated to just include the pairing fix now. I agree let's get this added in and I will look into a better approach to fix the other issue. Zach gave me some ideas, but I might not be able to look into it until later this week or maybe early next week. If you approve Colin, I can merge it in.

@rschwant rschwant merged commit 50772b3 into NOAA-CSL:develop Nov 12, 2024
5 checks passed
@rschwant
Copy link
Collaborator Author

Just to keep track of these updates. I did think about when this bug would cause problems. If your model top is lower than your flight tracks, the code without this PR would incorrectly choose the top of the model values for all data. This would be a pretty obvious error that we have not gotten yet because we haven't tested the code for stratospheric field campaign data yet. So anyone using stratospheric field campaign data should use the current develop branch or MELODIES MONET code > version 0.1. This update, fixes this problem and adds a warning if you are trying to pair data from obs above your model top, which is not recommended anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants