renepickhardt / pickhardtpayments

python package to showcase, test and build your own version of Pickhardt Payments
Apache License 2.0
42 stars 14 forks source link

- integrated the property `bee_line_distance` to the `Channel` class #27

Open RohitRathore1 opened 2 years ago

RohitRathore1 commented 2 years ago
renepickhardt commented 2 years ago

Hey conceptually this is exactly what will be needed to add latency as an additional feature and thanks a lot for your suggestion. I am a bit reluctant to merge at this point:

  1. adding an additional field to the ChannelFields class breaks compatibility with c-lightning which we currently have
  2. I would love to see tests first, that the bee line distance does actually work well to predict latency.

Once we have such results I will be very happy to merge this and integrate this into the library.

RohitRathore1 commented 2 years ago

Hey conceptually this is exactly what will be needed to add latency as an additional feature and thanks a lot for your suggestion. I am a bit reluctant to merge at this point:

  1. adding an additional field to the ChannelFields class breaks compatibility with c-lightning which we currently have
  2. I would love to see tests first, that the bee line distance does actually work well to predict latency.

Once we have such results I will be very happy to merge this and integrate this into the library.

@renepickhardt Thanks for the feedback. These two changes are fine till now? Am I right? Now I will focus on bee_line_distance. When it will work correctly then we can move forward but before bee line distance I wanted to confirm for above these two properties that I have done correctly or not.

renepickhardt commented 2 years ago

Yes if those Features work one could add them like this. The actual feature engineering would later Happen in the uncertainty Networks class though