✅

Hi, I am the SymPy bot. I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

• physics.units
  • Corrected the bug in multiplication between prefix and quantity, e.g. milli * W = 1 which should be W/1000. (#24909 by @HieuLCM)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.13.

#### References to other Issues or PRs
Fixes #24832 

#### Brief description of what is fixed or changed
Previously, when multiplying a prefix and a quantity, the code would return 1 if the product of their scale factors was equal to 1. This resulted in incorrect output in some cases, such as when multiplying "milli" and "W", which would evaluate to 1 instead of W/1000. 
To fix this issue, the code has been updated to only return 1 when a prefix is multiplied by another prefix. When a prefix is multiplied by a quantity, the product of their scale factors is no longer checked for equality to 1.

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers. Formerly, `log(-x)` incorrectly gave `-log(x)`.

* physics.units
  * Corrected a semantical error in the conversion between volt and statvolt which
    reported the volt as being larger than the statvolt.

or if no release note(s) should be included use:

NO ENTRY

See https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more
information on how to write release notes. The bot will check your release
notes automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->

* physics.units
  * Corrected the bug in multiplication between prefix and quantity, e.g. milli * W = 1 which should be W/1000.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

I don't understand why fact=1 for milli*watt; I understand it for milli*kilo. You should add a test that demonstrates that this now works, e.g.

 from sympy.physics.units import milli, W  # this is probably already imported in a unit's test file (in the sympy/physics/units/tests folder)
assert milli*W != 1

Also, when fact=1 I wonder if we should be returning S.One instead of int(1).

Benchmark results from GitHub Actions

Lower numbers are good, higher numbers are bad. A ratio less than 1
means a speed up and greater than 1 means a slowdown. Green lines
beginning with + are slowdowns (the PR is slower then master or
master is slower than the previous release). Red lines beginning
with - are speedups.

Significantly changed benchmark results (PR vs master)

Significantly changed benchmark results (master vs previous release)

       before           after         ratio
     [8bd31f66]       [a3068416]
     <sympy-1.12rc1^0>                 
-         101±1ms       63.8±0.9ms     0.63  integrate.TimeIntegrationRisch02.time_doit(10)
-        99.4±1ms       63.6±0.8ms     0.64  integrate.TimeIntegrationRisch02.time_doit_risch(10)

Full benchmark results can be found as artifacts in GitHub Actions
(click on checks at the top of the PR).

I don't understand why fact=1 for milli*watt; I understand it for milli*kilo. You should add a test that demonstrates that this now works, e.g.

 from sympy.physics.units import milli, W  # this is probably already imported in a unit's test file (in the sympy/physics/units/tests folder)
assert milli*W != 1

Also, when fact=1 I wonder if we should be returning S.One instead of int(1).

By default, the W (watt) has 1000 as scale_factor (related to SI system). Therefore, when taking the multiplication, the old code returns 1.
In this case, I ensure the mul only returns one if the two factors are prefixes.

About the test, thank you for reminding me, I will add the test.

It's a relief to see this fixed. Thanks! In retrospect, we could have returned fact instead of S.One, but it is clear that it is correct with what you have done.