Issue 5862047: New Stochastic Propogation Models for ns-3

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Issue 5862047: New Stochastic Propogation Models for ns-3

Can't Edit
Can't Publish+Mail
Start Review

Created:
12 years, 1 month ago by and.kirill

Modified:
12 years ago

Reviewers:
Tom Henderson, ns-3-reviews

CC:
ns-3-reviews_googlegroups.com

Visibility:
Public.

More Reviews

Description

New Stochastic Propogation Models for ns-3

Patch Set 1 #

Total comments: 5

Patch Set 2 : Replacing an old Jakes model with a new one #

Created: 12 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+349 lines, -327 lines)			Patch
M	src/propagation/examples/main-propagation-loss.cc	View	1	2 chunks	+2 lines, -2 lines	0 comments	Download
A	src/propagation/model/jakes-process.h	View	1	1 chunk	+92 lines, -0 lines	0 comments	Download
A	src/propagation/model/jakes-process.cc	View	1	1 chunk	+134 lines, -0 lines	0 comments	Download
M	src/propagation/model/jakes-propagation-loss-model.h	View	1	2 chunks	+20 lines, -104 lines	0 comments	Download
M	src/propagation/model/jakes-propagation-loss-model.cc	View	1	2 chunks	+12 lines, -221 lines	0 comments	Download
A	src/propagation/model/propagation-cache.h	View		1 chunk	+86 lines, -0 lines	0 comments	Download
M	src/propagation/wscript	View	1	2 chunks	+3 lines, -0 lines	0 comments	Download

Messages

Total messages: 1

Expand All Messages | Collapse All Messages

Tom Henderson

12 years ago (2012-04-24 13:04:57 UTC) #1

The code looks fine to me, I think the missing items for merge (that would also
help a review):

- example usage of the model (perhaps even a simple toy example in examples/
directory)
- CHANGES.html that documents any changed behavior on the old (narrowband) model
as well as summarizing new API and capabilities
- .rst text, perhaps summarizing your wns3 poster?
- a regression test that spot checks your correlation properties and channel
gain 
- various licenses missing (as noted in the review)

please add ns-3-reviews@googlegroups.com to this issue; I don't think I saw the
original mail

http://mail.codereview.appspot.com/5862047/diff/1/src/propagation/model/discr...
File src/propagation/model/discrete-fourier-transform.h (right):

http://mail.codereview.appspot.com/5862047/diff/1/src/propagation/model/discr...
src/propagation/model/discrete-fourier-transform.h:1: #ifndef NS3_DFT_H_
File is missing copyright

http://mail.codereview.appspot.com/5862047/diff/1/src/propagation/model/discr...
src/propagation/model/discrete-fourier-transform.h:10: * \brief O(N^2) DFT
a bit more verbose documentation please

http://mail.codereview.appspot.com/5862047/diff/1/src/propagation/model/wideb...
File src/propagation/model/wideband-doppler-process.cc (right):

http://mail.codereview.appspot.com/5862047/diff/1/src/propagation/model/wideb...
src/propagation/model/wideband-doppler-process.cc:1: #include
"wideband-doppler-process.h"
missing license

http://mail.codereview.appspot.com/5862047/diff/1/src/propagation/model/wideb...
File src/propagation/model/wideband-doppler-process.h (right):

http://mail.codereview.appspot.com/5862047/diff/1/src/propagation/model/wideb...
src/propagation/model/wideband-doppler-process.h:1: #ifndef
WIDEBAND_DOPPLER_PROCESS_H_
license

http://mail.codereview.appspot.com/5862047/diff/1/src/propagation/model/wideb...
src/propagation/model/wideband-doppler-process.h:12: {
class documentation

Expand All Messages | Collapse All Messages