Skip to content

Add diffusion coefficient quantities and related units.#322

Merged
iliekturtles merged 1 commit into
iliekturtles:masterfrom
crystal-growth:add_diffusion_coefficient
Aug 9, 2022
Merged

Add diffusion coefficient quantities and related units.#322
iliekturtles merged 1 commit into
iliekturtles:masterfrom
crystal-growth:add_diffusion_coefficient

Conversation

@crystal-growth

Copy link
Copy Markdown
Contributor

Should resolve #321

I'm not sure completely about singular for "stokes" unit, it seems it's always used in plural form.

@iliekturtles

Copy link
Copy Markdown
Owner

Thanks for all these PRs! I'll try to work through them ASAP but it will likely take me some days.

@crystal-growth

Copy link
Copy Markdown
Contributor Author

Thank you! Please take your time!

@iliekturtles iliekturtles left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I've started my review. What's the best way to suggest changes? I've done all three for this PR.

diff --git a/src/si/diffusion_coefficient.rs b/src/si/diffusion_coefficient.rs
index 37ec6b4..959d4ac 100644
--- a/src/si/diffusion_coefficient.rs
+++ b/src/si/diffusion_coefficient.rs
@@ -13,20 +13,19 @@ quantity! {
         Z0,     // amount of substance
         Z0>;    // luminous intensity
     units {
-        @square_meter_per_second: prefix!(none); "m² · s⁻¹",
-            "square meter per second", "square meters per second";
-        @square_centimeter_per_second:  prefix!(centi) * prefix!(centi); "cm² · s⁻¹",
-            "square centimeter per second", "square centimeters per second";    
-        @square_millimeter_per_second:  prefix!(milli) * prefix!(milli); "mm² · s⁻¹",
-            "square millimeter per second", "square millimeters per second";                
-        @square_micrometer_per_second:  prefix!(micro) * prefix!(micro); "µm² · s⁻¹",
-            "square micrometer per second", "square micrometers per second"; 
-        @square_nanometer_per_second:  prefix!(nano) * prefix!(nano); "nm² · s⁻¹",
-            "square nanometer per second", "square nanometers per second";  
-        @stokes:  prefix!(centi) * prefix!(centi); "St",
-            "stokes", "stokes";    
-        @centistokes:  prefix!(centi) * prefix!(centi) * prefix!(centi); "cSt",
-            "stokes", "stokes";                                     
+        @square_meter_per_second: prefix!(none); "m┬▓/s", "square meter per second",
+            "square meters per second";
+        @square_centimeter_per_second: prefix!(centi) * prefix!(centi); "cm┬▓/s",
+            "square centimeter per second", "square centimeters per second";
+        @square_millimeter_per_second: prefix!(milli) * prefix!(milli); "mm┬▓/s",
+            "square millimeter per second", "square millimeters per second";
+        @square_micrometer_per_second: prefix!(micro) * prefix!(micro); "┬╡m┬▓/s",
+            "square micrometer per second", "square micrometers per second";
+        @square_nanometer_per_second: prefix!(nano) * prefix!(nano); "nm┬▓/s",
+            "square nanometer per second", "square nanometers per second";
+        @stokes: prefix!(centi) * prefix!(centi); "St", "Stokes", "Stokes";
+        @centistokes: prefix!(centi) * prefix!(centi) * prefix!(centi); "cSt", "centistokes",
+            "centistokes";
         }
 }
 
@@ -35,7 +34,7 @@ mod test {
     storage_types! {
         use crate::num::One;
         use crate::si::quantities::*;
-        use crate::si::time as t;      
+        use crate::si::time as t;
         use crate::si::area as area;
         use crate::si::diffusion_coefficient as dc;
 
@@ -49,15 +48,15 @@ mod test {
 
         #[test]
         fn check_units() {
-            test::<t::second, area::square_meter, dc::square_meter_per_second>();
-            test::<t::second, area::square_centimeter, dc::square_centimeter_per_second>();
-            test::<t::second, area::square_millimeter, dc::square_millimeter_per_second>();
-            test::<t::second, area::square_micrometer, dc::square_micrometer_per_second>();
-            test::<t::second, area::square_nanometer, dc::square_nanometer_per_second>();
-            test::<t::second, area::square_centimeter, dc::stokes>();
-            test::<t::second, area::square_millimeter, dc::centistokes>();
+            test::<area::square_meter, t::second, dc::square_meter_per_second>();
+            test::<area::square_centimeter, t::second, dc::square_centimeter_per_second>();
+            test::<area::square_millimeter, t::second, dc::square_millimeter_per_second>();
+            test::<area::square_micrometer, t::second, dc::square_micrometer_per_second>();
+            test::<area::square_nanometer, t::second, dc::square_nanometer_per_second>();
+            test::<area::square_centimeter, t::second, dc::stokes>();
+            test::<area::square_millimeter, t::second, dc::centistokes>();
 
-            fn test< T: t::Conversion<V>, A: area::Conversion<V>, DC: dc::Conversion<V>>() {
+            fn test<A: area::Conversion<V>, T: t::Conversion<V>, DC: dc::Conversion<V>>() {
                 Test::assert_approx_eq(&DiffusionCoefficient::new::<DC>(V::one()),
                     &(Area::new::<A>(V::one())
                         / Time::new::<T>(V::one())));

Comment thread src/si/diffusion_coefficient.rs Outdated
Comment thread src/si/diffusion_coefficient.rs Outdated
Comment thread src/si/diffusion_coefficient.rs Outdated
@iliekturtles

Copy link
Copy Markdown
Owner

One more thing to add is that if you rebase on top of the latest master the 1.43.0 tests will stop failing.

@crystal-growth crystal-growth force-pushed the add_diffusion_coefficient branch from 1cf6d33 to 2a9a0db Compare August 7, 2022 21:08
@crystal-growth

Copy link
Copy Markdown
Contributor Author

I think that suggestions in PR will be the easiest option.
I've applied your suggestions and rebased this branch to new master.

Resolves iliekturtles#321

Co-authored-by: Mike Boutin <mike.boutin@gmail.com>
@crystal-growth crystal-growth force-pushed the add_diffusion_coefficient branch from 2a9a0db to 94f1b8c Compare August 8, 2022 12:27
@iliekturtles iliekturtles merged commit d55d64a into iliekturtles:master Aug 9, 2022
@iliekturtles

Copy link
Copy Markdown
Owner

Thanks for this PR! I'll be trying to work through the other PRs this week.

@crystal-growth crystal-growth deleted the add_diffusion_coefficient branch September 3, 2022 07:51
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.

Add new quantity: DiffusionCoefficient

2 participants