search

kconnour / pyRT_DISORT

A Python package for helping to compute input arrays to DISORT.
https://kconnour.github.io/pyRT_DISORT/
BSD 3-Clause "New" or "Revised" License
14 stars 5 forks source link

Example code for extracting UMU and PHI using SkyAngles #7

Closed mjwolff closed 3 months ago

mjwolff commented 2 years ago

please update documentation to show clear example of how the single column for UMU and PHI should be extracted for the call to disort.disort, i.e.

angles = sky_image(ina, azi0, ema_array, azi_array) mu = angles.mu mu0 = angles.mu0 phi = angles.phi phi0 = angles.phi0 UMU = mu[0,:] UMU0 = mu0 PHI = phi[0,:] PHI0 = phi0

kconnour commented 2 years ago

You should not hesitate to contact me if you're getting stuck for a while on something I can fix quickly...

This one I'm still confused about. I went to great lengths to explain the meaning of output property shapes in the Angles documentation... is it still unclear? I don't mind adding this code to the example if you think it'll be helpful. Alternatively, I could just retool the class to only hold on to data from a single beam at once so UMU0 and PHI0 are both scalars and UMU and PHI are 1D arrays. The current version of Angles, which can handle data of any shape, is much more computationally efficient than making an object for each incident beam (it wouldn't matter for the rover case, but for a realistic IUVS image that's 133x206 pixels I'd need to make ~27,000 of these objects---one for each pixel). But, if you think the human time cost will be significant, it may be worth it to make things simpler, plus the time needed to make many of these objects would be much less than the time a solver needs to fit some parameter to each pixel. I guess a 3rd alternative would be to have the aforementioned alternative be the default, but I have a class that behaves like the current version of Angles for those that want to use it (me). 

I had a grand idea that all of the public classes/functions (where applicable) could handle input of any data shape but the more I think about it the harder it seems to create (and I can't even be sure who would want that). Having all classes assumed to be for a single incident beam would be a lot simpler for me to create. If so, I can make that clear on the homepage.

Any input you can provide is helpful... even hearing you say RT people would not expect the current behavior tells me to rethink things.

kconnour commented 3 months ago

This is obsolete with the new API.