open-atmos / PyPartMC

Python (and C++) interface to PartMC with Jupyter/Python, Julia and Matlab examples
https://open-atmos.github.io/PyPartMC/
GNU General Public License v3.0
22 stars 9 forks source link

support for AeroMode instantiation with sampled mode type (incl. fixes in spec file logic) + sampled and mono mode types coverage in unit tests (in test_aero_[mode,state,dist].py) #357

Closed slayoo closed 5 months ago

slayoo commented 6 months ago

@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?

jcurtis2 commented 6 months ago

@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.

slayoo commented 6 months ago

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

jcurtis2 commented 6 months ago

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

slayoo commented 6 months ago

Thanks Jeff!

slayoo commented 6 months ago

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

jcurtis2 commented 6 months ago

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.

slayoo commented 5 months ago

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