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

Showing how to manually cancel units #451

Merged
merged 8 commits into from
Jun 4, 2021

Conversation

KronosTheLate
Copy link
Contributor

@KronosTheLate KronosTheLate commented May 29, 2021

The doc-section on Unit cancellation currently only states that units with differing SI-prefixes don't get cancelled. In this PR, I have added documentation for ways to cancel the units manually, which I have found very useful.

@codecov-commenter
Copy link

codecov-commenter commented May 29, 2021

Codecov Report

Merging #451 (0cff5bf) into master (cc3adef) will decrease coverage by 0.01%.
The diff coverage is 91.66%.

❗ Current head 0cff5bf differs from pull request most recent head 17407cb. Consider uploading reports for the commit 17407cb to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #451      +/-   ##
==========================================
- Coverage   83.84%   83.83%   -0.02%     
==========================================
  Files          16       16              
  Lines        1337     1342       +5     
==========================================
+ Hits         1121     1125       +4     
- Misses        216      217       +1     
Impacted Files Coverage Δ
src/display.jl 95.23% <91.66%> (-0.97%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc3adef...17407cb. Read the comment docs.

added the statement that `m/m` is automatically canceled
julia> using Unitful, Unitful.DefaultSymbols

# Converting the units to be equal with `uconvert`:
julia> uconvert(m, 1km) / 1m
Copy link
Contributor

Choose a reason for hiding this comment

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

At that point, wouldn't you rather write ustrip(1km, m)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the output would be

julia> ustrip(m, 1km)/1m
1000.0 m^-1

, whereas the intended output was to cancel the km with the m. I see that this could have been made more clear by dividing with e.g. 2cm.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant just ustrip(m, 1km), which is equivalent to uconvert(m, 1km) |> NoUnit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, right. Because the point of the /1m is not to remove the unit, it is part of the expression. I'll make this more clear in the example by changing it to 2.5m

docs/src/conversion.md Outdated Show resolved Hide resolved
@cstjean
Copy link
Contributor

cstjean commented Jun 1, 2021

As an aside, why do you care about unit cancelation? In my code, I either use ustrip when I have to pass a number to a function that can't deal with units, or I leave the units be...

@KronosTheLate
Copy link
Contributor Author

As an aside, why do you care about unit cancelation? In my code, I either use ustrip when I have to pass a number to a function that can't deal with units, or I leave the units be...

For the case that made me create this PR, was printing - I foundit annoying to get 1km/m when I expect 1000.

This commit changes the expression from which units are shortened to 1km/2.5m, to make clear that the denomenator is part of the expression.

It also fixed a typo (`NoUnit -> NoUnits`)
@KronosTheLate
Copy link
Contributor Author

I sense that I think this is more important than you who have reviewed it think. I also see that including both of the first two examples seems more like documenting the fact that you can use units as functions for conversion. Maybe the example should be changed to only the NoUnits one, to make this section more concise?

It seems like some of the examples did not sit well with some of the contributers. I see their point completely, and in this PR I have made the example minimal and clearer. #LessIsMore
docs/src/conversion.md Outdated Show resolved Hide resolved
docs/src/conversion.md Outdated Show resolved Hide resolved
KronosTheLate and others added 3 commits June 3, 2021 14:19
I added a reference to the NoUnits type (hopefully - I don't know how to chech that the reference actually works as expected).

I also changed the example to the more complicated one, to make it less trivial and hopefully better showcase the usefullness.
Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>
@giordano giordano merged commit 9b77235 into PainterQubits:master Jun 4, 2021
hustf added a commit to hustf/Unitfu.jl that referenced this pull request Sep 1, 2022
* Use the `:fancy_exponent` IO context property to override the behavior of fancy exponents (PainterQubits#446)

* Showing how to manually cancel units (PainterQubits#451)

* Showing how to manually cancel units

* Update conversion.md

added the statement that `m/m` is automatically canceled

* Remove contrived examples, fixed typo

* More clear example

This commit changes the expression from which units are shortened to 1km/2.5m, to make clear that the denomenator is part of the expression.

It also fixed a typo (`NoUnit -> NoUnits`)

* Shorten example

It seems like some of the examples did not sit well with some of the contributers. I see their point completely, and in this PR I have made the example minimal and clearer. #LessIsMore

* Added @ref, and more interesting example

I added a reference to the NoUnits type (hopefully - I don't know how to chech that the reference actually works as expected).

I also changed the example to the more complicated one, to make it less trivial and hopefully better showcase the usefullness.

* Update docs/src/conversion.md

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* As per the latest review, which got outdated

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* fix example `isa(1m, Length)` (PainterQubits#454)

`isa(1m, Length)` did not work after `using Unitful` but the corrected version works.

* Elaborate on UnitfulRecipes in readme (PainterQubits#456)

If I had known this would support plot axes with units, I would not have started down the road to PainterQubits#455.
Maybe this elaboration will help someone else.

[skip ci]

* Add link to UnitfulBuckinghamPi in README (PainterQubits#442)

I just registered the [UnitfulBuckinghamPi.jl](https://github.com/rmsrosa/UnitfulBuckinghamPi.jl) package. It solves for the adimensional groups in a list of Unitful parameters (quantities, units, dimensions, or combinations thereof).

This PR adds a link to the package in the README.

[skip ci]

* Fix doctests (PainterQubits#464)

* deg2rad and rad2deg with "units" (PainterQubits#459)

* deg2rad and rad2deg

* v1.9

* Update src/pkgdefaults.jl

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

* Update README.md

Mentioned NaturallyUnitful.jl

* Update .gitignore (PainterQubits#482)

* Update .gitignore

* Update .gitignore

Removed binary stuff

[skip ci]

* Use aggressive constprop for `^(::AbstractQuantity, ::Rational)` (PainterQubits#487)

* Calculate correct `eltype` when multiplying `StepRangeLen` by `Units`. (PainterQubits#485)

* Prevent promotion of StepRangeLen eltype when multiplying by unit

* Release 1.9.1

* Fix multiplication of range and quantity (PainterQubits#489)

* Remove unnecessary Bool-AbstractQuantity multiplication methods (PainterQubits#491)

* made Bool-Quantity multiplication symmetric

* added tests for Bool-Quantity multiplication

* Remove unnecessary multiplication methods for Bool, AbstractQuantity

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Remove comment from previous Bool, AbstractQuantity methods

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Add NaN, -Inf tests for multiplication by false

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Resolve method ambiguity (PainterQubits#495)

* Fix fastmath trig functions (PainterQubits#497)

* make `norm` use `norm` rather than `abs` (PainterQubits#500)

* make `norm` use `norm` rather than `abs`

* support broadcasting on ranges (PainterQubits#501)

* Attempt to allow preferunits to work with non-pure units (PainterQubits#478)

* Attempt to allow preferunits to work with non-pure units

Should fix PainterQubits#457. Prior to this change, attempts to use something that
looked like "preferunits(C/ms)" would result in strange behavior, without
throwing any sort of errors. Now, that should work nicely.

* Warn user when preferunits causes redundant units

Issue a warning when preferunits creates redundant units, which could stop
it from sucessfully simplifying certain quantities.

* Fix new upreferred behavior for non-dimensional quantities

* Add tests for preferunits changes

* Update test/runtests.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Fix preferunits tests

* Apply suggestions from code review

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

* Fix issues added by removing excess arrays

* No longer use string for key while checking for units of different scales

* Update test/runtests.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Update runtests.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>
Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

* Unit construction macro now defines docs for units (PainterQubits#476)

* Unit construction macro now defines docs for units

See PainterQubits#436

* Add documentation capabilities to most defined macros

This lets all unit and dimension definition macros repect doc strings
set in unit definition. Additionally, it automatically defines
documentation for derived units using power-of-ten prefixes.

* Add documentation for some units in pkgdefaults

Documentation has been added for all base units and many other SI units, as
well as adding a special case to the documentation so that kg and
documented as being the SI base unit.

* Allow documentation for log scales and units

* Rewrite documentation format for all units through "Liter"

Additionally, changed how the prefixed unit documentation is written; this
now means that prefixed units made by other modules should be properly
documented (tested with UnitfulAstro).

* Added documentation for all remaining units

Documentation is still required for log scales

* Update src/pkgdefaults.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Update src/pkgdefaults.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Update src/pkgdefaults.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Update src/pkgdefaults.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Apply suggestions from code review

Added descriptions of why certain alternate symbols are used (q for electron charge, minute for minutes)

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Remove excess newlines; add names to docstrings

* Add NoDims documentation

* Standardized number and equation format among quantity and unit docs

* Standardize format of dimensions in unit docs

* Apply suggestions from code review

Various minor fixes and changes, mostly typos and format changes.

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Switch from inline to block code blocks in new docstrings.

Also standardize docstrings to `2π` for 2 pi.

* Add automatic documentation for everything defined by the dimension macro

Also removed documentation from the pieces of log units that currently
don't have any sort of automatic documentation generation system in place.

* Fix documentation issues with two different symbols for vacuum permittivity constant.

* Minor formatting fixes

* Use block quotes for prefixed units' doc strings

* Update src/types.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Make automatic documentation optional

Also (optionally) generate automatic documentation for derived dimensions.

* Add dimension info in automatic prefixed unit docs.

* Change method for adding dimensions to prefixed unit documentation

Now no longer tries to `eval` stuff while running the macro.

* Fixes to documentation and code structure for autodocs for various macros

* Apply suggestions from code review

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Various format fixes and liters documentation changes.

* Fix missing optional argument causing test failure

* Remove no-longer-necessary NullLogger for liter and kilogram docs

* Remove unnecessary autodocs arguments for non-SI-prefixed units

* Apply suggestions from code review

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Fix invalid links being created in non-Unitful documentation

* Add spaced out column for autogen parameter where possible

* Apply suggestions from code review

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Update src/user.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Add tests for autodocs

* Add test for prefixed reference units

* Hopefully fix docstrings and docstring tests on Julia-1.0

* Apply suggestions from code review

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* More documentation changes from code review

Also removed documentation ability for logunit and logscale (as these should be another pull request)

* Apply suggestions from code review

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* fix range eltype (PainterQubits#503)

* Fix some major invalidations to improve compile times (PainterQubits#509)

* Fix some major invalidations to improve compile times

Unitful.jl was flagged as a major invalidator of compilation downstream. Test:

```julia
# From: https://timholy.github.io/SnoopCompile.jl/stable/snoopr/
using SnoopCompile
invalidations = @Snoopr begin
    using DifferentialEquations

    function lorenz(du,u,p,t)
     du[1] = 10.0(u[2]-u[1])
     du[2] = u[1]*(28.0-u[3]) - u[2]
     du[3] = u[1]*u[2] - (8/3)*u[3]
    end
    u0 = [1.0;0.0;0.0]
    tspan = (0.0,100.0)
    prob = ODEProblem(lorenz,u0,tspan)
    alg = Rodas5()
    tinf = solve(prob,alg)
end;

trees = SnoopCompile.invalidation_trees(invalidations);

@show length(SnoopCompile.uinvalidated(invalidations)) # show total invalidations

show(trees[end]) # show the most invalidated method
```

This method won the prize for the absolute most invalidations 🎉. But I think the bigger issue is that it simply doesn't follow Julia semantics. It fixes the types for issue PainterQubits#127 in a way that gives a stricter type than Julia would do in the normal cases (which is why the invalidation occurs).

After this PR, heterogeneous arrays of numbers with Quantity in there act normally, and compile times are back to normal. Here's a showcase of it being normal:

```julia

using Unitful, Test
m = u"m"
cm = u"cm"

b = Union{Complex,Float64}[0 + 0im, 0.0]
@test b + b == b
@test b .+ b == b
@test eltype(b+b) === Number

b = Number[0 + 0im, 0.0]
@test b + b == b
@test b .+ b == b
@test eltype(b+b) === Number

b = [0.0, 0.0m]
@test b + b == b
@test b .+ b == b
@test eltype(b+b) === Number
```

* Update promotion.jl

* Release v1.10.0

* Fix PainterQubits#465 for isapprox with complex arrays (PainterQubits#468)

* deg2rad and rad2deg

* v1.9

* Update src/pkgdefaults.jl

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

* fix PainterQubits#465

* test, v1.9.1

* Update src/quantities.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* positive test

* Update test/runtests.jl

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Fix `range` implementation on Julia master and resolve method ambiguities (PainterQubits#514)

* _range(start, ::Nothing, ::Nothing, len)

* _range(::Nothing, ::Nothing, stop, len)

* Add tests for ranges with complex elements

* Use modern  syntax

* Remove unnecessary `<:Real`

* Fix range(start::Quantity{BigFloat}; step::Quantity{BigFloat}; length)

* Fix for missing _rangestyle on 1.8

* Remove `real`

* Use `step` everywhere (instead of `st`)

* Fix range(;step, stop, len) on Julia master

* Fix range(;stop, step, length) for mixed number/quantity

* Add some newlines and comments

* Rename _range_step_stop_length → _unitful_step_stop_length

To avoid confusion with Base.range_step_stop_length

* Refactor _range(start, ::Nothing, stop, length)

* Rename 3-arg `Base._range` → `_unitful_start_stop_length` (`Base._range` always has 4 arguments)
* Move promotion to `_unitful_start_stop_length`

* Refactor _range(start, step, ::Nothing, length)

* Resolve method ambiguity on Julia ≥ 1.7

* Simplify implementation

* Use modern `where` syntax for `colon`

* Add two-argument colon for dimensionless quantities

* Remove unnecessary code related to encoding of μ (PainterQubits#511)

* Add tests for encoding of μ

* Remove unnecessary code

* Use U+03BC everywhere

* Same changes for U+025B/U+03B5

* Fix printing of `StepRangeLen` with complex elements (PainterQubits#513)

* Enable `zero` for arrays with non-concrete eltype (PainterQubits#516)

* Release 1.11.0 (PainterQubits#519)

* Delete export of `convertr`, `convertrp` (PainterQubits#530)

* Delete export of `convertr`

There is no `convertr`, so don't export it.

* Don't export convertrp

It's not defined

* Add `cispi` and `sincospi` (PainterQubits#533)

* Add `modf` (PainterQubits#539)

* Add `modf` for dimensionless quantities

* Add tests for `modf`

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>

* Add link to UnitfulChainRules.jl to README (PainterQubits#542)

* modified:   Project.toml v1.10.1 reflect Unitful, add ConstructionBase
modified:   src/pkgdefaults.jl   Use new macros, deg rad change
modified:   test/runtests.jl     Add ConstructionBase

* modified:   Project.toml   v1.11.0

Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
Co-authored-by: KronosTheLate <61620837+KronosTheLate@users.noreply.github.com>
Co-authored-by: Sebastian Stock <42280794+sostock@users.noreply.github.com>
Co-authored-by: Jeff Fessler <JeffFessler@users.noreply.github.com>
Co-authored-by: Ricardo Rosa <rmsrosa@gmail.com>
Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
Co-authored-by: Mo8it <76752051+Mo8it@users.noreply.github.com>
Co-authored-by: Samuel Buercklin <SBuercklin@users.noreply.github.com>
Co-authored-by: Oscar Smith <oscardssmith@gmail.com>
Co-authored-by: Alexander <alexander@plav.in>
Co-authored-by: Luke Bemish <lukebemish@gmail.com>
Co-authored-by: Christopher Rackauckas <accounts@chrisrackauckas.com>
Co-authored-by: adkabo <61999878+adkabo@users.noreply.github.com>
Co-authored-by: Rashid Rafeek <rashidrafeek@gmail.com>
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.

5 participants