Adjust airplane altitude calculation in LUT generation#789
Merged
Conversation
Collaborator
|
This is definitely a correct identified bug and a good patch - thanks Wonseok! We need to think carefully before merging for historic sRTMnet trainings (we might need to add something into the sRTMnet class too). |
Collaborator
|
Rebased and brought back up to speed. I fully expect all checks to pass. Given that this is a clear correction to the physics, I think we can merge. Previous hesitancy stemmed from backwards compatibility...but given how quickly we're iterating on dev on the 6c physics, I'm inclined to just bring this in. Also, testing with the 3c emulator shows that there are not deleterious consequences from incorporating. Any resistance to merging this in? |
Collaborator
|
Thanks, @WonseokChoi0! Finally merging this one in. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change is about the definition of "airplane altitude".
isofit intended to generate LUT with a fixed airplane altitude (asl) which is described as H1ALT in MODTRAN template file.
However, 6S defines the airplane altitude as "above ground level" and internally add ground altitude to the airplane altitude.
This caused simulating atmospheric transfer functions at different airplane altitudes as follows:
In the above cases, the H1ATL was set to 1.17 km in isofit, but we found that 6S simulated spectra with different airplane altitudes like 1.17, 2.13, and 3.33.
To correct this issue, I have added one line of code in sixs_s.py calculating above ground level by subtracting surface_elevation_km from observer_altitude_km. When the surface_elevation_km was greater than the observer_altitude_km, I set the AGL to its minimum value of 0.01 km.