Skip to content

Change architecture to separate parser logic from server logic (untested)#5

Open
jacobdunefsky wants to merge 12 commits into
mainfrom
task2-arch-fix-untested
Open

Change architecture to separate parser logic from server logic (untested)#5
jacobdunefsky wants to merge 12 commits into
mainfrom
task2-arch-fix-untested

Conversation

@jacobdunefsky

Copy link
Copy Markdown
Collaborator

I separated the parser logic from the server logic (thank you Kai for his comment on #24 (openalto/ietf-hackathon#24)). Now, the parser lives in a G2Parser class that is used in the server. Note that parser code can easily be swapped out, if we want to develop another parser, as long as the parser implements a method construct_from_flows(flows_list).

emiapwil and others added 7 commits March 20, 2022 11:14
Signed-off-by: Kai Gao <emiapwil@gmail.com>
Signed-off-by: Kai Gao <emiapwil@gmail.com>
Signed-off-by: Kai Gao <emiapwil@gmail.com>
Signed-off-by: Kai Gao <emiapwil@gmail.com>
Refactored from estimate_throughput.py
Now, works with any Parser object that supports a function construct_from_flows(flows_list) that returns a dict with "A" matrix, "c" vector, and "RTT".
Comment thread src/alto/server/estimate_throughput.py Outdated
Comment thread src/alto/server/estimate_throughput.py Outdated
Comment thread src/alto/server/estimate_throughput.py Outdated
Comment thread src/alto/server/estimate_throughput.py Outdated
Comment thread src/alto/server/estimate_throughput.py Outdated
Comment thread src/alto/server/estimate_throughput.py Outdated
Comment thread src/alto/server/g2parser.py Outdated
Comment thread src/alto/server/g2parser.py Outdated
Comment thread src/alto/app/estimator.py Outdated
Comment thread src/alto/app/estimator.py
Comment thread src/alto/app/estimator.py Outdated
Comment thread src/alto/app/estimator.py Outdated
Comment thread src/alto/app/estimator.py Outdated
Comment thread src/alto/app/estimator.py Outdated
Comment thread src/alto/app/estimator.py
Currently, only `G2_MININET` is supported.
"""

SOLVER = "jensen"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "num" is a better name for the solver, as this solver uses the "Network Utility Maximization (NUM)" model to do the prediction.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never see the name is changed. Re-opened this conversation.

Comment thread src/alto/app/estimator.py Outdated
Comment thread src/alto/app/estimator.py
Comment thread src/alto/app/estimator.py Outdated
@fno2010

fno2010 commented May 5, 2022

Copy link
Copy Markdown
Member

Overall, the PR looks good to me. But some files are still using hard TAB indents.

* This script assumes that the configuration files are located at
"input/g2.conf" and "output/input_routing.conf". If this is not the case,
then change the variables g2fp_str and infp_str
* This script estimates RTT by summing up the delays of each link. This is

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest changing slightly the language. Instead of:

"This is not accurate!!! But, since..."

Say something like:

"This is for now a first order approximation, since queuing and processing delay are not taken into account."

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants