Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

- integrated the property bee_line_distance to the Channel class #27

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RohitRathore1
Copy link

@RohitRathore1 RohitRathore1 commented Jun 8, 2022

  • integrated the property latency to the OracleLightningNetwork class

- integrated the property `latency` to the `OracleLightningNetwork` class

Signed-Off-By: Rohit Singh Rathaur <[email protected]>
@renepickhardt
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants