Conversation
|
@eulaliesa, you should provide an overview of the pull request, rather than the details of the last commit. Here is an example: This PR adds a column |
…atellite_Health # Conflicts: # src/prx/rinex_nav/test/test_evaluate.py
| "I": (0, 3), | ||
| "S": (0, 63), | ||
| } | ||
| for const, hf in zip(query["constellation"], query["health_flag"]): |
There was a problem hiding this comment.
You could also try assigning the range to two new columns with something like
for constellation,(ub, lb) in health_valid_ranges.items():
query.loc[query.constellation == constellation, "upper_bound"] = ub
query.loc[query.constellation == constellation, "lower_bound"] = lb
assert (query["health_flag"] <= query["upper_bound"]).all()
Sorry for any syntax errors, I did not actually try to tun this :)
Again, replacing what effectively is a loop over rows by a small number of vectorized operations will give us a speedup here.
There was a problem hiding this comment.
Could we use one of the existing test files here?
There was a problem hiding this comment.
Oh wait I guess we need it because the existing files do not have unhealthy satellites?
There was a problem hiding this comment.
Indeed, I chose this test file because the satellite G27 had an unhealthy flag. The test was made to check how the position error was influenced by an unhealthy satellite.
|
|
||
| per_sat_eph_query = per_sat_eph_query.groupby("orbit_type").apply(evaluate_orbit) | ||
| per_sat_eph_query = per_sat_eph_query.reset_index(drop=True) | ||
| per_sat_eph_query["health_flag"] = extract_health_flag_from_query(per_sat_eph_query) |
There was a problem hiding this comment.
I recommend a shorter
query["health_flag"] = query["health"]
query.loc[query.constellation=="C", "health_flag"] = query.loc[query.constellation=="C", "SatH1"]
Vectorized pandas operations tend to be (much) faster than looping over rows :).
I'd then also remove test_extract_health_flag_from_query since extracting the correct health flag values is already tested by test_compute_health_flag.
|
@eulaliesa I am not sure I understand "directly use RINEX OBS files instead of pre-processed .prx files for generating query data." in the PR description - the CSV file (what does ".prx files" refer to?) is generated in the very end, so the query is always generated from observation timestamps. What am I missing? |
|
@jtec I modified |
|
@eulaliesa Thanks for addressing the comments :) I took the liberty to update the PR description with the wording suggested by @plutonheaven. We try to keep the PR descriptions as clear as possible, since they are typically what future developers will look at to better understand a particula commit. PR descriptions are versioned, so feel free to roll back the changes I made if you disagree :) Did you have a chance to run Line 36 in aaef973 Running those benchmarks automatically in CI has been on our list for a while but did not make it into the code yet. |
|
@jtec I ran |
… 202506_Satellite_Health
…atellite_Health # Conflicts: # src/prx/rinex_nav/test/test_evaluate.py
3 participants
This PR adds a column health_flag to prx output. It does so by choosing the correct data from parsed RINEX NAV files depending on the constellation of an observation.
A flag equal to 0 means that the satellite is usable. If different from 0, depending on the constellation, some signals may not be usable. The interpretation of non-zero values are left to the user of the prx file.
Tests have been included to validate the correct assignment of the health flag value. Also, the benefits of considering the satellite health status is demonstrated in a positioning solution computed with observations containing an unhealthy satellite (G27 on 2024-01-01)