vtsuperdarn / davitpy

DEPRECATED The DaViT Python project
http://vtsuperdarn.github.com/davitpy/
GNU General Public License v3.0
37 stars 59 forks source link

plotFan error #347

Closed mts299 closed 6 years ago

mts299 commented 6 years ago

When following the example in the Fan.py file: davitpy.pydarn.plotting.fan.plotFan(date, radar, param='power', show=False, png=True, gsct=True, fileType='fitacf') where the parameters are set by the follwing inputs: python generatefanplots.py sas 2016 03 03 20 20 1 The following error is raised: Traceback (most recent call last): File "/usr/lib64/python2.7/multiprocessing/process.py", line 258, in _bootstrap self.run() File "/usr/lib64/python2.7/multiprocessing/process.py", line 114, in run self._target(*self._args, **self._kwargs) File "generatefanplots.py", line 59, in plot_one_scan_fan_plots fileType='fitacf') File "/usr/lib64/python2.7/site-packages/davitpy-0.8-py2.7-linux-x86_64.egg/davitpy/pydarn/plotting/fan.py", line 354, in plotFan bbox = myFig.gca().get_axes().get_position()

With @asreimer help (who was able to run the script successfully with no errors), it seems the error is caused by using a newer version of matplotlib: My versions:

numpy.version '1.14.1' scipy.version '1.0.0' matplotlib.version '2.1.2' @asreimer versions: In [2]: matplotlib.version Out[2]: '1.5.1' In [4]: numpy.version Out[4]: '1.11.0' In [6]: scipy.version Out[6]: '0.13.3'

aburrell commented 6 years ago

The new matplotlib is breaking a lot of coding projects :-P I think we should put out the fixes as a bugfix, since a lot of users will encounter this.

mts299 commented 6 years ago

I have heard in my lab from a couple of students how the code is breaking when they upgrade matplotlib. I don't mind tackling this project and others when I come across them :) I would also love to improve this file as it seems to be in rough shape for flexibility.

aburrell commented 6 years ago

Please do! Tag me when you have a pull request ready, and I’ll help with testing.

On 8 Mar 2018, at 11:21, mts299 notifications@github.com wrote:

I have heard in my lab from a couple of students how the code is breaking when they upgrade matplotlib. I don't mind tackling this project and others when I come across them :) I would also love to improve this file as it seems to be in rough shape for flexibility.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/vtsuperdarn/davitpy/issues/347#issuecomment-371558468, or mute the thread https://github.com/notifications/unsubscribe-auth/AGuC_tfKOLYm4T29zPODRUnv349nld5Aks5tcWiggaJpZM4Si9XJ.

ksterne commented 6 years ago

I'll 2nd on testing once you've got a fix, at least I'll try my best as time allows.

mts299 commented 6 years ago

Thanks for all the support in helping me test the fixes. Currently, I got it to read in radarDataOpen record similar to what the rti plotting method does. However, I get this nice logg warning message for some of the Radars: WARNING:root: Accuracy on height calculation (5.0) not reached quick enough. Returning nan, nan. This message causes the program to core dump :(

Any thoughts on what is going on? It does it only (so far) with Clyde River and Rankin Inlet data, which is what I want to process.

Also, side note, what is the desire of using logging in this library? Typical, best practice for python is to raise exceptions and only log at library system level when in debugging mode.

Thanks for the help! Hopefully I can do a pull request soon :)

mts299 commented 6 years ago

@aburrell - Can you please explain to me why you changed the termination condition in the pull request #274 for the while loop in calcFieldPnt in radFov.py? It was:

if abs(xAlt - dictOut['distAlt']) <= 0.5 or n > 2: return dictOut['distLat'], dictOut['distLon']

Now:

new_hdel = abs(xalt - geo_dict['distAlt']) if new_hdel <= htol: break if abs(new_hdel - hdel) < 1.0e-3: n = maxn When I changed line the condition to include n > 2 like before it worked for the Clyde River radar and Rankin Inlet. However, I can understand where the reason not to have in n > 2 in a numerical simulation.

Also, this line

hdel = new_hdel does not seem correct to me either, if I am to assume we are performing a numerical approximation. When I ran the current code with data on Clyde, I will can the warning that it couldn't read the height fast enough and at that point n = 3 which seems to be not enough steps for most things to converge too.

If you have any papers/books I can read to understand this algorithm better, please let me know :)

Also, can I get access to the github? I would like to push my code to the branch I made so I can test it on other machines with other data.

aburrell commented 6 years ago

Good questions @mts299, I'll take a stab at answering them.

The return conditions for the algorithm changed when I introduced the Chisham virtual height model, which treats 1.5-hop paths differently. I worked with Gareth to implement his model, settling on a more relaxed tolerance for 1.5-hop paths. In this case I decided to return nan because this code is not only run to produce fields of view, and in these cases the desired outcome is to get a set of virtual heights. In this instance, if one or more of the instances fail and the rest succeed, the results are easily understood and filtered.

I thought a recent pull request I submitted (it may or may not have been pushed in the last few weeks) corrected this for fan plots, by introducing a flag that tells the algorithm to skip the tolerance test.

In the cases you list, the tolerance is probably failing because the range gates you want virtual heights for follow 2.5-hop or greater paths. The virtual height models aren't designed to work at these distances and so their results are not good.

As far as I know, there's no reference for the standard model. The reference for the Chisham model is in the code, but here it is again: Chisham, G., T. K. Yeoman, and G. J. Sofko (2008), Mapping ionospheric backscatter measured by the SuperDARN HF radars Part 1: A new empirical virtual height model, Annales Geophysicae, 26(4), 823–841, doi:10.5194/angeo-26-823-2008.

As for getting access to the github, do you mean that you want to be a davitpy developer?

mts299 commented 6 years ago

@aburrell - Thanks! I think this makes sense because on my output a good chunk of the data points did succeed but when they failed it caused core dumps.

I changed your nan returns to raise exceptions instead because that is the pythonic thing to do :) If you want to keep the code running after this failure in the model, then one just needs to use a try-except block. I hope this is okay and makes sense?

Is there something in the code to detect these types of range gates that use 2.5 - hop? Or have a default behavior of returning what we have already if n = maxn and model == 'IS'?

mts299 commented 6 years ago

Sorry forgot to answer the last bit; developer access would be nice :) I don't mind contributing to the software since I will be using it and help with any issues/ testing pull requests.

aburrell commented 6 years ago

Although I'm a big python advocate, I'm still a C programmer at heart, so I'm happy to have changes to make things more pythonic. Just make sure that the other bits that access this code have the try/except case in, as appropriate!

So there's not a lot around to detect hops above 1.5. Since most radars have range gates that extend to a max of 75 gates, it's only an issue for a negligible portion of the data. I have some code that would apply this type of logic in my FoV routine (pydarn.proc.fov.update_backscatter), but I don't recall if you can run it independent of determining the field of view. It would be easy enough to extract, however, should you wish to do so.

Another problem with that is, even if you know they are >=2.5-hop, the VH models do not model these hops (check out the reference material). The reason they don't is that the radars used in each model don't observe enough of them. For operational purposes, it wouldn't be the worst thing to label any hop that fails the tolerance test as 2.5. It would be interesting to investigate their characteristics.

aburrell commented 6 years ago

Ah, here's the pull request that fixes this in the develop branch (master branch was already fixed): #344

It's still waiting to be merged.

aburrell commented 6 years ago

Not sure how to make you a developer. @ksterne probably has this knowledge?

mts299 commented 6 years ago

@aburrell - No worries. I can tell from how the code is written in davitpy; most people are C programmers :p I can make the changes as I got through the code. I will double check to make sure it doesn't break current code, but I need you and @ksterne to test it more extensively when I make a pull request as I am still new davitpy.

I will take a look at the FOV code and see if I can adapt it and get your opinion on it to make sure it is scientifically sound.

If you want I could add some logging information in to record any possible characteristics when this failure occurs? I would like to have code to detect these failures and handle them correctly without the user telling it what to do.

I will look at the pull request and test it out for you. However, if we come up with a slicker idea, we may want to use that instead :)

ksterne commented 6 years ago

It is done. I'd suggest fixing the problem first, and then if you want to go through and overhaul it that would be later. Good to hear you're looking at the Warning...returning nan messages those have been an annoyance (I don't mean that in a bad way @aburrell!), though not enough for me to deal with. It'll be a bit cleaner if those are moved to somewhere else. It's likely that I'm not using the code correctly and I should learn how to use it better in order to not get the warnings!

aburrell commented 6 years ago

Logging levels for subroutines are always a pain, since we want the warnings in some situations but not others. I think my default is to throw out a warning more than some of the other developer :-P Might be worth having a chat as a group, at some point.

100% behind fixing the problem first, and then making it slick and shiny. Very happy to have you onboard @mts299 !

mts299 commented 6 years ago

Fixed the error and passing files into plotFan. I also fixed a bug when there is no data provided for a given radar.

There seems to be an infinite loop with trying to obtain data from the database when there is no data in the database for that radar, but I will put this problem into another issue.

Note: There is another branch called: enhancement-fanplot_from_file this may be looking at issue I just fixed, but that branch is 850 commits behind :( Maybe we should delete it if pull request #354 is sufficient enough.

Calling this issue closed!