isce-framework / s1-reader

Sentinel-1 reader
Apache License 2.0
27 stars 12 forks source link

WIP ability to load orbit from annotation XML #41

Open LiangJYu opened 2 years ago

LiangJYu commented 2 years ago

This PR address #40 and add:

To do:

hfattahi commented 2 years ago

Thank you @LiangJYu , It looks good. I gave it a try and for one case I got the following error message from our isce3 orbit constructor:

ValueError: non-uniform spacing between state vectors encountered - interval between state vector at position 1 and state vector at position 2 is 9.999999 s, expected 10.000001 s

complaining that the orbit is not uniform. We know that we have a very strict threshold in isce3 about checking the orbit uniform sampling. Let's discuss if we should allow users to relax that threshold to be able to load some not very precise orbits at their own risk.

LiangJYu commented 2 years ago

Thank you @LiangJYu , It looks good. I gave it a try and for one case I got the following error message from our isce3 orbit constructor:

ValueError: non-uniform spacing between state vectors encountered - interval between state vector at position 1 and state vector at position 2 is 9.999999 s, expected 10.000001 s

complaining that the orbit is not uniform. We know that we have a very strict threshold in isce3 about checking the orbit uniform sampling. Let's discuss if we should allow users to relax that threshold to be able to load some not very precise orbits at their own risk.

@hfattahi what do you think about modifying the ISCE3 orbit constructors to take a relaxed threshold? It doesn't appear too complicated to do in ISCE3. Here's what I think needs to be done:

hfattahi commented 2 years ago

Thank you @LiangJYu , It looks good. I gave it a try and for one case I got the following error message from our isce3 orbit constructor:

ValueError: non-uniform spacing between state vectors encountered - interval between state vector at position 1 and state vector at position 2 is 9.999999 s, expected 10.000001 s

complaining that the orbit is not uniform. We know that we have a very strict threshold in isce3 about checking the orbit uniform sampling. Let's discuss if we should allow users to relax that threshold to be able to load some not very precise orbits at their own risk.

@hfattahi what do you think about modifying the ISCE3 orbit constructors to take a relaxed threshold? It doesn't appear too complicated to do in ISCE3. Here's what I think needs to be done:

  • Utilize the iscClose() that is overload to accepts an user specified threshold.
  • Modify the Orbit constructor and getOrbitTime to accept a user threshold while defaulting to current threshold

There can be two approaches to solve this issue I guess. One would be to allow irregular state vector sampling and let the orbit interpolator handle things. I believe we already have an open issue in isce3 repo and it will eventually come in.

The other solution is what you suggested. It is simpler but a bit risky. However, the users of these APIs are really advanced developers and they should know what they are doing if they decide to relax the threshold. I'm ok if we allow for a threshold to be passed to the orbit constructor. By default it won't be needed and would use the current strict threshold. Let's discuss on isce3 repo.