search

georust / rinex

RINEX and GNSS data processing :artificial_satellite:
Apache License 2.0
76 stars 19 forks source link

Context improvements #209

Closed gwbres closed 6 months ago

gwbres commented 7 months ago

This PR improves and simplifies the RnxContext structure. It makes it more efficient to represent what is required in GNSS and RINEX post processing. For example, it allows now to preprocess all types of products we support and generate new products. It slightly reworks the way we operate the RnxContext structure and removes a big memcopy that was previously required in previous version, making the initial phase of the program more efficient.

gwbres commented 6 months ago

Hello @lnicola

I'm trying to improve the RnxContext structure substancially but I need your help to figure out one method specifically.

To give more context: RnxContext is not correctly defined today, in the sense it does not hold internal references to inner RINEX or SP3 files. The consequence of this, is we need to Clone() the RINEX/SP3 objects we have just parsed, when forming RnxContext, which is quite a heavy operation that you ideally want to avoid.

With this PR i'm trying to fix that, mostly by introducing a lifetime to RnxContext. I am very close to a solution.

=======================

Well I found rapidly a solution to this, it turns out the inner type must be &mut Ref. This is no longer a problem, but I'm having a hard time providing a real usage of this new definition. I'm not even sure that is entirely "feasible" in Rust..

lnicola commented 6 months ago

Sorry, I didn't have a chance to look at this yesterday, and now I'm pretty confused. Even when looking 91664b2569283092dcd60d6755394b37e677ea4b and only the rinex crate, I get quite a lot of build errors, so I'm not sure it's the right commit:

image

lnicola commented 6 months ago

But yeah, looking at that pattern, it might need a Rc<RefCell<..>>, which is a bit awkward to work with. Are you sure that BlobData shouldn't "own" Rinex and Sp3? Can a Rinex value be a part of more than one BlobData?

gwbres commented 6 months ago

But yeah, looking at that pattern, it might need a Rc<RefCell<..>>, which is a bit awkward to work with. Are you sure that BlobData shouldn't "own" Rinex and Sp3? Can a Rinex value be a part of more than one BlobData?

Thank you once again for all your input, it turns out it is the "natural" conclusion I came with this morning, that I have injected with the latest commits.

The conclusion is we have a solution to avoid this heavy memcopy, but it requires to slightly modify the way we operate/build/define the context at high level (in the applicatation itself)