Should units_option(simplify = FALSE) simplify units when numerator and denominator are the same?
#355
Comments
|
simplify tends to create a lot of problems so I usually check for it to be off (I would love to be able to "lock" it to FALSE to prevent users from causing all kinds of bugs). In this particular case, it doesn't cause any problems for my package but I indeed wonder if this was intended. |
|
This was unintended, part of @billdenney's #335, which solves an issue with logarithmic units. I really dislike making yet another exception for handling logarithmic units, but maybe we should rethink that? Thoughts, @billdenney? Also, @henningte @ilikegitlab could you please elaborate what kind of problems does this create? |
|
thanks for the explanation. I see that this is a difficult issue. My main problem is that sometimes, quantities with different units, but identical numerator and denominator, really are different things and this difference gets lost when the unit is always simplified, but can be important when doing calculations (and I think this is the main reason why there is the option to turn auto-simplification off?). For example, carbon content may be given as mass content (unit g/g) or molar content (unit mol/mol) and while simplification does not produce inconsistencies on the level of units, it can produce inconsistencies/ambiguities during calculations: library(units)
#> udunits database from C:/Users/henni/AppData/Local/R/win-library/4.3/units/share/udunits/udunits2.xml
library(PeriodicTable)
# assume a sample consisting of 5 mol H and 2 mol C
C_mol <- units::set_units(5, value = "mol", mode = "standard")
H_mol <- units::set_units(2, value = "mol", mode = "standard")
C_mass <- C_mol * units::set_units(PeriodicTable::mass("C"), value = "g/mol", mode = "standard")
H_mass <- H_mol * units::set_units(PeriodicTable::mass("H"), value = "g/mol", mode = "standard")
total_mol <- C_mol + H_mol
total_mass <- C_mass + H_mass
C_rel_mol <- C_mol/total_mol
C_rel_mass <- C_mass/total_mass
# now: try to convert back
C_rel_mass * total_mol # corresponds to (g mol)/g (where the two grams refer to different things)
#> 6.772655 [mol]
C_rel_mol * total_mol # corresponds to mol
#> 5 [mol]Created on 2023-09-05 with reprex v2.0.2 Session infosessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#> setting value
#> version R version 4.3.1 (2023-06-16 ucrt)
#> os Windows 11 x64 (build 22621)
#> system x86_64, mingw32
#> ui RTerm
#> language (EN)
#> collate German_Germany.utf8
#> ctype German_Germany.utf8
#> tz Europe/Berlin
#> date 2023-09-05
#> pandoc 3.1.1 @ C:/Program Files/RStudio/resources/app/bin/quarto/bin/tools/ (via rmarkdown)
#>
#> ─ Packages ───────────────────────────────────────────────────────────────────
#> package * version date (UTC) lib source
#> cli 3.6.1 2023-03-23 [1] CRAN (R 4.3.1)
#> digest 0.6.33 2023-07-07 [1] CRAN (R 4.3.1)
#> evaluate 0.21 2023-05-05 [1] CRAN (R 4.3.1)
#> fastmap 1.1.1 2023-02-24 [1] CRAN (R 4.3.1)
#> fs 1.6.3 2023-07-20 [1] CRAN (R 4.3.1)
#> glue 1.6.2 2022-02-24 [1] CRAN (R 4.3.1)
#> htmltools 0.5.6 2023-08-10 [1] CRAN (R 4.3.1)
#> knitr 1.43 2023-05-25 [1] CRAN (R 4.3.1)
#> lifecycle 1.0.3 2022-10-07 [1] CRAN (R 4.3.1)
#> PeriodicTable * 0.1.2 2017-08-29 [1] CRAN (R 4.3.0)
#> Rcpp 1.0.11 2023-07-06 [1] CRAN (R 4.3.1)
#> reprex 2.0.2 2022-08-17 [1] CRAN (R 4.3.1)
#> rlang 1.1.1 2023-04-28 [1] CRAN (R 4.3.1)
#> rmarkdown 2.24 2023-08-14 [1] CRAN (R 4.3.1)
#> rstudioapi 0.15.0 2023-07-07 [1] CRAN (R 4.3.1)
#> sessioninfo 1.2.2 2021-12-06 [1] CRAN (R 4.3.1)
#> units * 0.8-1 2022-12-10 [1] CRAN (R 4.3.1)
#> withr 2.5.0 2022-03-03 [1] CRAN (R 4.3.1)
#> xfun 0.40 2023-08-09 [1] CRAN (R 4.3.1)
#> yaml 2.3.7 2023-01-23 [1] CRAN (R 4.3.0)
#>
#> [1] C:/Users/henni/AppData/Local/R/win-library/4.3
#> [2] C:/Program Files/R/R-4.3.1/library
#>
#> ──────────────────────────────────────────────────────────────────────────────Note: I'm not saying the 'units' package should take care of such ambiguities (this is why I have written elco), but it would be easier to keep track of such things if there was no auto-simplification of units. Since this is a breaking change (breaking code in 'elco' which currently relies on non-simplified units), I was interested whether this is how it should be (to me it feels a bit inconsistent since you can opt-out of auto-simplification only in the special case where numerator and denominator are different). I see that this is a difficult issue and that you could rightly say that this should be no business of the 'units' package. It would be good to know whether this behavior is permanent now. |
|
@henningte , The issue you're having is similar to the issue that I was bringing up in #134. There are two different quantities represented by the same unit (g carbon/g atmospheric gas, I'm guessing). I agree that I introduced a bug here, and I think that it's resolution is how simplification is handled in cases with identical units dividing or inverse-multiplying. If |
|
We will revert to the previous behavior for sure (trying to preserve the log-stuff fixed by @billdenney in #335), and we will add the proper tests to ensure that That said, as @billdenney pointed out, this has to do with the discussion in #134. And as I said there, I repeat: I don't think you are using the best solution here. Grams are grams, and grams over grams is a ratio, and it's unitless. I believe your trouble only starts when you try to define conversions to moles. A mole is not a unit. We can call it a parametric unit at best. So it doesn't make any sense to define a conversion of moles to grams (and udunits2 doesn't). It does make sense to define conversions from mole of substance to grams. E.g., creating new units as So even if |
|
I agree that we should revert this. It's a limitation of the upstream udunits library that doesn't distinguish between |
|
I think that the only change required to make this work again is at this line: Line 98 in 3cd552f I have an option to make it respect the description in |
|
Although I agree with the spirit of the discussion, I would like to counter one sentence before it get taken as a consensus opinion: "Grams are grams, and grams over grams is a ratio, and it's unitless." I would not think of gram CO2 per gram air as unitless. This has nothing to do with moles, but with that we can have different physical quantities in the same units. A unit system does not necessarily have to know about this peculiarity, but it would be nice if it doesn't actively interfere with real-world data and practice. |
|
The PR I just made resolves the change with one line of code partially modified (and some tests). |
I agree with you that this would be nice to have, but is outside the scope of this package. The units of If you want to handle the |
|
@Enchufa2 and @billdenney, thanks for your detailed discussion. I have read #134 as @billdenney suggested and I see that this is a better solution for 'elco'. I wasn't too experienced with the units package (and in general) when I started working on it and one always learns new things. So thanks for that! (I have already adapted 'elco' so that it defines custom units for each chemical element now and also conversion factors between masses and amounts) Also thanks for your plan to restore the original behavior. I can confirm that #356 solves at least my issue: library(units)
#> udunits database from C:/Users/henni/AppData/Local/R/win-library/4.3/units/share/udunits/udunits2.xml
units_options(simplify = TRUE)
set_units(1, kg/kg)
#> 1 [1]
units_options(simplify = NA)
set_units(1, kg/kg)
#> 1 [kg/kg]
units_options(simplify = FALSE)
set_units(1, kg/kg)
#> 1 [kg/kg]Created on 2023-09-06 with reprex v2.0.2 Session infosessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#> setting value
#> version R version 4.3.1 (2023-06-16 ucrt)
#> os Windows 11 x64 (build 22621)
#> system x86_64, mingw32
#> ui RTerm
#> language (EN)
#> collate German_Germany.utf8
#> ctype German_Germany.utf8
#> tz Europe/Berlin
#> date 2023-09-06
#> pandoc 3.1.1 @ C:/Program Files/RStudio/resources/app/bin/quarto/bin/tools/ (via rmarkdown)
#>
#> ─ Packages ───────────────────────────────────────────────────────────────────
#> package * version date (UTC) lib source
#> cli 3.6.1 2023-03-23 [1] CRAN (R 4.3.1)
#> digest 0.6.33 2023-07-07 [1] CRAN (R 4.3.1)
#> evaluate 0.21 2023-05-05 [1] CRAN (R 4.3.1)
#> fastmap 1.1.1 2023-02-24 [1] CRAN (R 4.3.1)
#> fs 1.6.3 2023-07-20 [1] CRAN (R 4.3.1)
#> glue 1.6.2 2022-02-24 [1] CRAN (R 4.3.1)
#> htmltools 0.5.6 2023-08-10 [1] CRAN (R 4.3.1)
#> knitr 1.43 2023-05-25 [1] CRAN (R 4.3.1)
#> lifecycle 1.0.3 2022-10-07 [1] CRAN (R 4.3.1)
#> Rcpp 1.0.11 2023-07-06 [1] CRAN (R 4.3.1)
#> reprex 2.0.2 2022-08-17 [1] CRAN (R 4.3.1)
#> rlang 1.1.1 2023-04-28 [1] CRAN (R 4.3.1)
#> rmarkdown 2.24 2023-08-14 [1] CRAN (R 4.3.1)
#> rstudioapi 0.15.0 2023-07-07 [1] CRAN (R 4.3.1)
#> sessioninfo 1.2.2 2021-12-06 [1] CRAN (R 4.3.1)
#> units * 0.8-3 2023-09-06 [1] Github (billdenney/units@d57f54d)
#> withr 2.5.0 2022-03-03 [1] CRAN (R 4.3.1)
#> xfun 0.40 2023-08-09 [1] CRAN (R 4.3.1)
#> yaml 2.3.7 2023-01-23 [1] CRAN (R 4.3.0)
#>
#> [1] C:/Users/henni/AppData/Local/R/win-library/4.3
#> [2] C:/Program Files/R/R-4.3.1/library
#>
#> ────────────────────────────────────────────────────────────────────────────── |
|
@henningte, The example you're giving in your comment just above this one is not testing the same thing. I don't think that your code should have triggered the division code that I implemented (or if it does, then I'm very surprised-- I don't see any division occurring). This follows the documented behavior of the devtools::load_all("c:/git/Not_my_repositories/units/")
#> ℹ Loading units
#> udunits database from C:/git/Not_my_repositories/units/inst/share/udunits/udunits2.xml
units_options(simplify = TRUE)
set_units(1, kg)/set_units(1, kg)
#> 1 [1]
units_options(simplify = NA)
set_units(1, kg)/set_units(1, kg)
#> 1 [1]
units_options(simplify = FALSE)
set_units(1, kg)/set_units(1, kg)
#> 1 [kg/kg]Created on 2023-09-06 with reprex v2.0.2 |
|
you know the units package better than I do. All I can say is that your PR causes a different behavior for the example I gave to open the issue (compare the second code block in #355 (comment) and the code block in #355 (comment)). Or do I miss anything here? |
|
@henningte , I believe that is intentional. I think that you are referring to |
|
perhaps we misunderstand each other because I was not clear enough? I opened this issue not because I had problems with automatic unit simplification when dividing two different unit objects which have the same library(units)
#> udunits database from C:/Users/henni/AppData/Local/R/win-library/4.3/units/share/udunits/udunits2.xml
units_options(simplify = FALSE)
set_units(1, kg/kg)
#> 1 [kg/kg]Created on 2023-09-07 with reprex v2.0.2 Session infosessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#> setting value
#> version R version 4.3.1 (2023-06-16 ucrt)
#> os Windows 11 x64 (build 22621)
#> system x86_64, mingw32
#> ui RTerm
#> language (EN)
#> collate German_Germany.utf8
#> ctype German_Germany.utf8
#> tz Europe/Berlin
#> date 2023-09-07
#> pandoc 3.1.1 @ C:/Program Files/RStudio/resources/app/bin/quarto/bin/tools/ (via rmarkdown)
#>
#> ─ Packages ───────────────────────────────────────────────────────────────────
#> package * version date (UTC) lib source
#> cli 3.6.1 2023-03-23 [1] CRAN (R 4.3.1)
#> digest 0.6.33 2023-07-07 [1] CRAN (R 4.3.1)
#> evaluate 0.21 2023-05-05 [1] CRAN (R 4.3.1)
#> fastmap 1.1.1 2023-02-24 [1] CRAN (R 4.3.1)
#> fs 1.6.3 2023-07-20 [1] CRAN (R 4.3.1)
#> glue 1.6.2 2022-02-24 [1] CRAN (R 4.3.1)
#> htmltools 0.5.6 2023-08-10 [1] CRAN (R 4.3.1)
#> knitr 1.43 2023-05-25 [1] CRAN (R 4.3.1)
#> lifecycle 1.0.3 2022-10-07 [1] CRAN (R 4.3.1)
#> Rcpp 1.0.11 2023-07-06 [1] CRAN (R 4.3.1)
#> reprex 2.0.2 2022-08-17 [1] CRAN (R 4.3.1)
#> rlang 1.1.1 2023-04-28 [1] CRAN (R 4.3.1)
#> rmarkdown 2.24 2023-08-14 [1] CRAN (R 4.3.1)
#> rstudioapi 0.15.0 2023-07-07 [1] CRAN (R 4.3.1)
#> sessioninfo 1.2.2 2021-12-06 [1] CRAN (R 4.3.1)
#> units * 0.8-3 2023-09-06 [1] Github (billdenney/units@d57f54d)
#> withr 2.5.0 2022-03-03 [1] CRAN (R 4.3.1)
#> xfun 0.40 2023-08-09 [1] CRAN (R 4.3.1)
#> yaml 2.3.7 2023-01-23 [1] CRAN (R 4.3.0)
#>
#> [1] C:/Users/henni/AppData/Local/R/win-library/4.3
#> [2] C:/Program Files/R/R-4.3.1/library
#>
#> ──────────────────────────────────────────────────────────────────────────────(note this now behaves as I have expected, so my issue is solved --- this is all I wanted to say in my previous post) But originally, I did not consider division of two unit objects with the same library(units)
#> udunits database from C:/Users/henni/AppData/Local/R/win-library/4.3/units/share/udunits/udunits2.xml
units_options(simplify = FALSE)
set_units(1, kg)/set_units(1, kg)
#> 1 [kg/kg]Created on 2023-09-07 with reprex v2.0.2 Session infosessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#> setting value
#> version R version 4.3.1 (2023-06-16 ucrt)
#> os Windows 11 x64 (build 22621)
#> system x86_64, mingw32
#> ui RTerm
#> language (EN)
#> collate German_Germany.utf8
#> ctype German_Germany.utf8
#> tz Europe/Berlin
#> date 2023-09-07
#> pandoc 3.1.1 @ C:/Program Files/RStudio/resources/app/bin/quarto/bin/tools/ (via rmarkdown)
#>
#> ─ Packages ───────────────────────────────────────────────────────────────────
#> package * version date (UTC) lib source
#> cli 3.6.1 2023-03-23 [1] CRAN (R 4.3.1)
#> digest 0.6.33 2023-07-07 [1] CRAN (R 4.3.1)
#> evaluate 0.21 2023-05-05 [1] CRAN (R 4.3.1)
#> fastmap 1.1.1 2023-02-24 [1] CRAN (R 4.3.1)
#> fs 1.6.3 2023-07-20 [1] CRAN (R 4.3.1)
#> glue 1.6.2 2022-02-24 [1] CRAN (R 4.3.1)
#> htmltools 0.5.6 2023-08-10 [1] CRAN (R 4.3.1)
#> knitr 1.43 2023-05-25 [1] CRAN (R 4.3.1)
#> lifecycle 1.0.3 2022-10-07 [1] CRAN (R 4.3.1)
#> Rcpp 1.0.11 2023-07-06 [1] CRAN (R 4.3.1)
#> reprex 2.0.2 2022-08-17 [1] CRAN (R 4.3.1)
#> rlang 1.1.1 2023-04-28 [1] CRAN (R 4.3.1)
#> rmarkdown 2.24 2023-08-14 [1] CRAN (R 4.3.1)
#> rstudioapi 0.15.0 2023-07-07 [1] CRAN (R 4.3.1)
#> sessioninfo 1.2.2 2021-12-06 [1] CRAN (R 4.3.1)
#> units * 0.8-3 2023-09-06 [1] Github (billdenney/units@d57f54d)
#> withr 2.5.0 2022-03-03 [1] CRAN (R 4.3.1)
#> xfun 0.40 2023-08-09 [1] CRAN (R 4.3.1)
#> yaml 2.3.7 2023-01-23 [1] CRAN (R 4.3.0)
#>
#> [1] C:/Users/henni/AppData/Local/R/win-library/4.3
#> [2] C:/Program Files/R/R-4.3.1/library
#>
#> ──────────────────────────────────────────────────────────────────────────────This is what you tested --- if I understand correctly, this is yet another issue and, if my assumption is right, then your PR may solve both. So yes, I think the package now behaves as intended when dividing two unit objects with the same I think this is where we may have thought of two different cases, perhaps? |
…#356) * Keep units with identical units when `units_options(simplify = FALSE)` (Fix #355) * Restore deleted snaps * some refactoring * simplify test * bump version --------- Co-authored-by: Iñaki Úcar <[email protected]>
|
Reviewed and merged, thanks everyone! @edzer I bumped the version number. I think a patch release is in order. |
I have an issue regarding automatic unit conversion. In previous versions, the 'units' package did not automatically simplify units to the dimensionless unit (1) when numerator and denominator were the same, however, now it does. I wonder whether this is expected behavior?
Here is the example from #132 (comment), which works as expected:
Created on 2023-09-05 with reprex v2.0.2
Session info
However, if numerator and denominator are the same, no matter what
units_optionssimplifyis set to, the unit is simplified:Created on 2023-09-05 with reprex v2.0.2
Session info
This was not the case with version 0.8-1:
Created on 2023-09-05 with reprex v2.0.2
Session info
Is how
units_options(simplify = FALSE)andunits_options(simplify = NA)work in version 0.8-3 expected behavior?The text was updated successfully, but these errors were encountered: