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

Conversions Module arithmetic operator precedence issue #21

Open
cbowers1020 opened this issue Feb 20, 2020 · 5 comments
Open

Conversions Module arithmetic operator precedence issue #21

cbowers1020 opened this issue Feb 20, 2020 · 5 comments

Comments

@cbowers1020
Copy link

In the conversions module file conversions.py, there are many examples of using multiple division operators to achieve the final form of an equation (e.g., freespace loss).

In the case of the free_space_loss calculation (method: _free_space_loss), (C_VALUE / 4. / np.pi / f / d) ** 2 != (4 * np.pi * f * d) / C_VALUE) ** 2 (a more human readable format. Though the pycraf implementation ((C_VALUE / 4. / np.pi / f / d) ** 2) is mathematically correct, python does NOT evaluate the operators correctly. I have remedied this error locally, but I suspect (though have not verified) that the following methods in conversions.py are additionally affected by this mathematical implementation:

  • iso_eff_area
  • gamma_from_eff_area
  • eff_area_from_gain
  • antfactor_from_gain
  • efield_from_ptx
  • powerflux_from_ptx
  • powerflux_from_prx
  • prx_from_powerflux
  • t_a_from_prx_nu
  • t_a_from_powerflux_nu
  • prx_from_ptx
  • ptx_from_prx

This error does not affect any pyprop implementations and free space loss using that method reflects the actual calculation.

@bwinkel
Copy link
Owner

bwinkel commented Feb 20, 2020

Thanks a lot for your interest in pycraf!

I'm not sure, if I get your point, especially when you write that

python does NOT evaluate the operators correctly

Could you perhaps add an example of what gets incorrectly calculated?

@cbowers1020
Copy link
Author

Sure. After closer inspection, I think I overstated the error. The actual problem is as follows.

In calculating free space path loss (which is how I found this error), the equation would be: FSPL = ((4. * pi * distance * frequency) / C_Constant) ^ 2

This equation is implemented in conversions.py in method _free_space_loss as:
pycraf_FSPL = (C_Constant / 4. / pi / frequency / distance) ** 2,
which evaluates to 1 / FSPL. I guess this is not a problem, but this value when returned to the wrapper function, free_space_loss, is treated as the regular, what I call, FSPL value.

Additionally, though this doesn't affect the calculation, the wrapper method free_space_loss switches frequency and distance when passing them into the implementation method _free_space_loss.

I hope this explanation makes a bit more sense. I apologize for the earlier confusion.

Screen Shot 2020-02-20 at 7 06 12 PM

@bwinkel
Copy link
Owner

bwinkel commented Feb 21, 2020

I see. Most RF engineers who I know prefer the FSPL (expressed in dB) to be a positive number (as it is a loss), whereas one can also find the definition, which you quote, where it is a negative decibel value. Personally, I have no strong feelings about this, but as the ITU-R recommendations, e.g., P.452, which are used in pycraf also follow the former definition, I'll leave the free_space_loss function as it is.

I'm sorry about the switch of frequency and distance in the function definition, which is very unfortunate indeed. I may fix this in a future release, although this is dangerous - if someone made use of the _free_space_loss in their software, it would break their code.

@cbowers1020
Copy link
Author

That's actually how I came upon this error--I was expecting a positive loss value and received a negative value.

Only for low frequencies or very short distances will the original pycraf implementation return a value greater than one and thus, the conversion to dB will result in a positive number.

Below is a sampling of calculationa in the wireless network I am simulating at various frequencies: 5GHz, 5MHz, and 5000 Hz.

I guess we are both ultimately dancing around the same value, so it doesn't really matter. With this equation though, (versus the pyprof free space loss calculation implementation: 92.5 + 20 * log10(frequency) + 20 * log10(distance) for GHz and km), it would be hard for the user to know what sign to expect.

Screen Shot 2020-02-21 at 7 28 33 AM

Screen Shot 2020-02-21 at 7 31 55 AM

Screen Shot 2020-02-21 at 7 32 09 AM

@bwinkel
Copy link
Owner

bwinkel commented Feb 26, 2020

I see. Then I defined it in the opposite way. I'll probably fix this in a future release, although it is always a bit painful for downstream developers if you change the API.

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

No branches or pull requests

2 participants