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

new particle_shape_and_density #1539

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tluettm
Copy link
Collaborator

@tluettm tluettm commented Mar 14, 2025

concept for new handling of particle attributes depended on shape (area, volume, mass, capacity...)

  • decoupled liquid and ice particles into different classes so that their shape can be changed separately
  • currently, spherical shape of liquid particles is assumed implicitly in a lot of routines. Not a problem right now, but plenty of opportunity for refactoring in the (far) future!
  • however, for ice particles all access to shape related variables should go through particle_shape_ice
    common parent class of spheres for liquid and ice spheres
    I realize that the use of static methods in the spheres class and definition to self.mass density in the initiation of sub classes, liquid and ice spheres, does not make much sense in the current iteration
  • is the access to the trivia functions implemented correctly by importing the entire trivia object?
  • the proposed change would require to rework mixed moments routine

In the future, we could move to a logic where the shape is assigned (and can be changed!) for all super particles individually instead of separating them into just two (liquid and ice) particles. However, more 'fancy' shape realtionships like shape habit prediction could also be used with the current seperation intwo two types

@staticmethod
def mass_to_volume(const, mass):
return (
Trivia.volume_of_density_mass(const, const.rho_i, mass)
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, this will not work with the current implementation of Formulae. Everything in the physics folder is expected to take form of one-liners without calls to other than NumPy math functions. It will choke both in the GPU backend (where Trivia. will not work in C code), but likely also in Numba CPU backend for it does not allow non-Numba class accessors.

@staticmethod
def area(const, radius):
return const.PI * const.FOUR * np.power(radius, const.TWO)

@staticmethod
def volume(const, radius):
def sphere_radius_to_volume(const, radius):
Copy link
Member

Choose a reason for hiding this comment

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

very good point!

def __init__(self, const, _):
pass

@staticmethod
Copy link
Member

Choose a reason for hiding this comment

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

It seems that supports_mixed_phase within IceSpheres does not really convey much - should always be True

@slayoo slayoo added the ice label Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants