PyPartMC icon indicating copy to clipboard operation
PyPartMC copied to clipboard

covering AeroMode instantiation with sampled mode type in unit tests (both in test_aero_mode and test_aero_dist)

Open slayoo opened this issue 1 year ago • 7 comments

slayoo avatar Apr 23 '24 17:04 slayoo

@jcurtis2, the issue with the new test_sampled test was that

         "size_dist": [
                        {"num_conc": num_concs},
                        {"diam": [1, 2, 3, 4]},
                    ],

and

         "size_dist": [
                        {"diam": [1, 2, 3, 4]},
                        {"num_conc": num_concs},
                    ],

are not the same, because we retrieve the single-element dictionaries based on their placement in the size_dist value (list), and not based on the key. Now, there are checks in place informing user if the order is wrong. I've also added a check depicting that analogous summation of num_conc works if the mode is part of a dist.

Shall we merge?

slayoo avatar May 17 '24 09:05 slayoo

@jcurtis2, the issue with the new test_sampled test was that

         "size_dist": [
                        {"num_conc": num_concs},
                        {"diam": [1, 2, 3, 4]},
                    ],

and

         "size_dist": [
                        {"diam": [1, 2, 3, 4]},
                        {"num_conc": num_concs},
                    ],

are not the same, because we retrieve the single-element dictionaries based on their placement in the size_dist value (list), and not based on the key. Now, there are checks in place informing user if the order is wrong. I've also added a check depicting that analogous summation of num_conc works if the mode is part of a dist.

Shall we merge?

Just want to note that the order here is different than the PartMC input file order:

!! The resulting size distribution is taken to be piecewise
!! constant in log-diameter coordinates.
!!
!! Example: a size distribution could be:
!! <pre>
!! diam 1e-7 1e-6 1e-5  # bin edge diameters (m)
!! num_conc 1e9 1e8     # bin number concentrations (m^{-3})
!! </pre>
!! This distribution has 1e9 particles per cubic meter with
!! diameters between 0.1 micron and 1 micron, and 1e8 particles
!! per cubic meter with diameters between 1 micron and 10 micron.

But if we have things to check order in PyPartMC, this should be fine.

jcurtis2 avatar May 17 '24 14:05 jcurtis2

hmm... let's then first try adding a test that would check if the diams are correctly being stored...

slayoo avatar May 17 '24 18:05 slayoo

Yeah, at least debugging with some print statements in the PartMC code (inserted in spec_file_read_size_dist https://github.com/compdyn/partmc/blob/e9451e51d845542bb4f586d2d194bf41e7f19af0/src/aero_mode.F90#L795-L875), looks like the diameter bins are incorrect now instead of the number concentrations. Both data arrays in this function are the same.

diameters 100.00000000000000 200.00000000000000 300.00000000000000
num conc 100.00000000000000 200.00000000000000 300.00000000000000

jcurtis2 avatar May 17 '24 18:05 jcurtis2

Thanks Jeff!

slayoo avatar May 17 '24 19:05 slayoo

BTW, do we have any mean of testing it from Python at this point? (indirectly by sampling from the dist?)

slayoo avatar May 17 '24 19:05 slayoo

BTW, do we have any mean of testing it from Python at this point? (indirectly by sampling from the dist?)

Not currently - doesn't appear to be anything PartMC function that will do something with it (not like number concentration where there is a function that just gets the total number concentration of a mode aero_mode_total_num_conc). So we can either sample particles and make sure they are within the allowable bin range or we could write something to expose the sample_radius and sample_num_conc of the aero_mode though like we do for other parts like char_radius.

jcurtis2 avatar May 17 '24 19:05 jcurtis2

Thanks @jcurtis2 Looks good to me, please confirm if OK to merge (and release)?

slayoo avatar May 22 '24 20:05 slayoo