-
Notifications
You must be signed in to change notification settings - Fork 113
ENUM refactor #216
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
base: staging
Are you sure you want to change the base?
ENUM refactor #216
Changes from all commits
1988f19
9f8968d
a85cd57
2d8efbd
0107f16
5c2752e
fd5f988
4aa88b2
72b7c19
7210a69
826d408
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| from enum import Enum | ||
|
|
||
|
|
||
| # Python 3.11 supports `StrEnum` that would make this a bit more concise to write | ||
| # https://docs.python.org/3/library/enum.html#enum.StrEnum | ||
| class InhibitedEnds(str, Enum): | ||
| NEITHER = 'Neither' | ||
| TOP = 'Top' | ||
| BOTTOM = 'Bottom' | ||
| BOTH = 'Both' | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| from enum import Enum | ||
|
|
||
|
|
||
| # Python 3.11 supports `StrEnum` that would make this a bit more concise to write | ||
| # https://docs.python.org/3/library/enum.html#enum.StrEnum | ||
| class MultiValueChannels(str, Enum): | ||
| MASS = 'mass' | ||
| MASS_FLOW = 'massFlow' | ||
| MASS_FLUX = 'massFlux' | ||
| REGRESSION = 'regression' | ||
| WEB = 'web' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| from enum import Enum | ||
|
|
||
|
|
||
| # Python 3.11 supports `StrEnum` that would make this a bit more concise to write | ||
| # https://docs.python.org/3/library/enum.html#enum.StrEnum | ||
| class SimAlertLevel(str, Enum): | ||
| """Levels of severity for sim alerts""" | ||
| ERROR = 'Error' | ||
| WARNING = 'Warning' | ||
| MESSAGE = 'Message' |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||
| from enum import Enum | ||||||
|
|
||||||
| # Python 3.11 supports `StrEnum` that would make this a bit more concise to write | ||||||
| # https://docs.python.org/3/library/enum.html#enum.StrEnum | ||||||
| class SimAlertType(str, Enum): | ||||||
| """Types of sim alerts""" | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| GEOMETRY = 'Geometry' | ||||||
| CONSTRAINT = 'Constraint' | ||||||
| VALUE = 'Value' | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| from enum import Enum | ||
|
|
||
|
|
||
| # Python 3.11 supports `StrEnum` that would make this a bit more concise to write | ||
| # https://docs.python.org/3/library/enum.html#enum.StrEnum | ||
| class SingleValueChannels(str, Enum): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is recommended to use singular form for classes AFAIK |
||
| TIME = 'time' | ||
| KN = 'kn' | ||
| PRESSURE = 'pressure' | ||
| FORCE = 'force' | ||
| VOLUME_LOADING = 'volumeLoading' | ||
| EXIT_PRESSURE = 'exitPressure' | ||
| D_THROAT = 'dThroat' | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| from enum import Enum | ||
|
|
||
|
|
||
| # Python 3.11 supports `StrEnum` that would make this a bit more concise to write | ||
| # https://docs.python.org/3/library/enum.html#enum.StrEnum | ||
| class Unit(str, Enum): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it might make sense to create a base class Unit and add unit specific classes as well , like ImpulseUnit, LenghtUnit etc. Wil l help to be specific and more constistent |
||
| # Angle | ||
| DEGREES = 'deg' | ||
|
|
||
| # Burn Rate Coefficient | ||
| METER_PER_SECOND_PASCAL_TO_THE_POWER_OF_N = 'm/(s*Pa^n)' | ||
| INCH_PER_SECOND_POUND_PER_SQUARE_INCH_TO_THE_POWER_OF_N = 'in/(s*psi^n)' | ||
|
|
||
| # Density | ||
| KILOGRAM_PER_CUBIC_METER = 'kg/m^3' | ||
| POUND_PER_CUBIC_INCH = 'lb/in^3' | ||
| GRAM_PER_CUBIC_CENTIMETER = 'g/cm^3' | ||
|
|
||
| # Force | ||
| NEWTON = 'N' | ||
| POUND_FORCE = 'lbf' | ||
|
|
||
| # Impulse | ||
| NEWTON_SECOND = 'Ns' | ||
| POUND_FORCE_SECOND = 'lbfs' | ||
|
|
||
| # Length | ||
| METER = 'm' | ||
| CENTIMETER = 'cm' | ||
| MILLIMETER = 'mm' | ||
| INCH = 'in' | ||
| FOOT = 'ft' | ||
|
|
||
| # Mass Flow | ||
| KILOGRAM_PER_SECOND = 'kg/s' | ||
| POUND_PER_SECOND = 'lb/s' | ||
| GRAM_PER_SECOND = 'g/s' | ||
|
|
||
| # Mass Flux | ||
| KILOGRAM_PER_SQUARE_METER_PER_SECOND = 'kg/(m^2*s)' | ||
| POUND_PER_SQUARE_INCH_PER_SECOND = 'lb/(in^2*s)' | ||
|
|
||
| # Mass | ||
| KILOGRAM = 'kg' | ||
| GRAM = 'g' | ||
| POUND = 'lb' | ||
| OUNCE = 'oz' | ||
| GRAM_PER_MOLE = 'g/mol' | ||
|
|
||
| # Nozzle Erosion Coefficient | ||
| METER_PER_SECOND_PASCAL = 'm/(s*Pa)' | ||
| METER_PER_SECOND_MEGAPASCAL = 'm/(s*MPa)' | ||
| THOUSANDTH_INCH_PER_SECOND_POUND_PER_SQUARE_INCH = 'thou/(s*psi)' | ||
|
|
||
| # Nozzle Slag Coefficient | ||
| METER_PASCAL_PER_SECOND = '(m*Pa)/s' | ||
| METER_MEGAPASCAL_PER_SECOND = '(m*MPa)/s' | ||
| INCH_POUND_PER_SQUARE_INCH_PER_SECOND = '(in*psi)/s' | ||
|
|
||
| # Pressure | ||
| PASCAL = 'Pa' | ||
| MEGAPASCAL = 'MPa' | ||
| POUND_PER_SQUARE_INCH = 'psi' | ||
|
|
||
| # Temperature | ||
| KELVIN = 'K' | ||
|
|
||
| # Time | ||
| SECOND = 's' | ||
|
|
||
| # Velocity | ||
| METER_PER_SECOND = 'm/s' | ||
| CENTIMETER_PER_SECOND = 'cm/s' | ||
| MILLIMETER_PER_SECOND = 'mm/s' | ||
| FOOT_PER_SECOND = 'ft/s' | ||
| INCH_PER_SECOND = 'in/s' | ||
|
|
||
| # Volume | ||
| CUBIC_METER = 'm^3' | ||
| CUBIC_CENTIMETER = 'cm^3' | ||
| CUBIC_MILLIMETER = 'mm^3' | ||
| CUBIC_INCH = 'in^3' | ||
| CUBIC_FOOT = 'ft^3' | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,11 @@ | |
| from scipy import interpolate | ||
|
|
||
| from . import geometry | ||
| from .simResult import SimAlert, SimAlertLevel, SimAlertType | ||
| from .enums.inhibitedEnds import InhibitedEnds | ||
| from .enums.simAlertLevel import SimAlertLevel | ||
| from .enums.simAlertType import SimAlertType | ||
| from .enums.unit import Unit | ||
| from .simResult import SimAlert | ||
| from .properties import FloatProperty, EnumProperty, PropertyCollection | ||
|
|
||
| class Grain(PropertyCollection): | ||
|
|
@@ -19,8 +23,8 @@ class Grain(PropertyCollection): | |
| geomName = None | ||
| def __init__(self): | ||
| super().__init__() | ||
| self.props['diameter'] = FloatProperty('Diameter', 'm', 0, 1) | ||
| self.props['length'] = FloatProperty('Length', 'm', 0, 3) | ||
| self.props['diameter'] = FloatProperty('Diameter', Unit.METER, 0, 1) | ||
| self.props['length'] = FloatProperty('Length', Unit.METER, 0, 3) | ||
|
|
||
| def getVolumeSlice(self, regDist, dRegDist): | ||
| """Returns the amount of propellant volume consumed as the grain regresses from a distance of 'regDist' to | ||
|
|
@@ -67,9 +71,9 @@ def getRegressedLength(self, regDist): | |
| endPos = self.getEndPositions(regDist) | ||
| return endPos[1] - endPos[0] | ||
|
|
||
| def getDetailsString(self, lengthUnit='m'): | ||
| def getDetailsString(self, Unit=Unit.METER): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is a good practice to keep args lowercase |
||
| """Returns a short string describing the grain, formatted using the units that is passed in""" | ||
| return 'Length: {}'.format(self.props['length'].dispFormat(lengthUnit)) | ||
| return 'Length: {}'.format(self.props['length'].dispFormat(Unit)) | ||
|
|
||
| @abstractmethod | ||
| def simulationSetup(self, config): | ||
|
|
@@ -103,17 +107,20 @@ class PerforatedGrain(Grain): | |
| geomName = 'perfGrain' | ||
| def __init__(self): | ||
| super().__init__() | ||
| self.props['inhibitedEnds'] = EnumProperty('Inhibited ends', ['Neither', 'Top', 'Bottom', 'Both']) | ||
| self.props['inhibitedEnds'] = EnumProperty('Inhibited ends', [InhibitedEnds.NEITHER, | ||
| InhibitedEnds.TOP, | ||
| InhibitedEnds.BOTTOM, | ||
| InhibitedEnds.BOTH]) | ||
| self.wallWeb = 0 # Max distance from the core to the wall | ||
|
|
||
| def getEndPositions(self, regDist): | ||
| if self.props['inhibitedEnds'].getValue() == 'Neither': # Neither | ||
| if self.props['inhibitedEnds'].getValue() == InhibitedEnds.NEITHER: | ||
| return (regDist, self.props['length'].getValue() - regDist) | ||
| if self.props['inhibitedEnds'].getValue() == 'Top': # Top | ||
| if self.props['inhibitedEnds'].getValue() == InhibitedEnds.TOP: | ||
| return (0, self.props['length'].getValue() - regDist) | ||
| if self.props['inhibitedEnds'].getValue() == 'Bottom': # Bottom | ||
| if self.props['inhibitedEnds'].getValue() == InhibitedEnds.BOTTOM: | ||
| return (regDist, self.props['length'].getValue()) | ||
| if self.props['inhibitedEnds'].getValue() == 'Both': | ||
| if self.props['inhibitedEnds'].getValue() == InhibitedEnds.BOTH: | ||
| return (0, self.props['length'].getValue()) | ||
| # The enum should prevent this from even being raised, but to cover the case where it somehow gets set wrong | ||
| raise ValueError('Invalid number of faces inhibited') | ||
|
|
@@ -135,7 +142,7 @@ def getCoreSurfaceArea(self, regDist): | |
|
|
||
| def getWebLeft(self, regDist): | ||
| wallLeft = self.wallWeb - regDist | ||
| if self.props['inhibitedEnds'].getValue() == 'Both': | ||
| if self.props['inhibitedEnds'].getValue() == InhibitedEnds.BOTH: | ||
| return wallLeft | ||
| lengthLeft = self.getRegressedLength(regDist) | ||
| return min(lengthLeft, wallLeft) | ||
|
|
@@ -145,9 +152,9 @@ def getSurfaceAreaAtRegression(self, regDist): | |
| coreArea = self.getCoreSurfaceArea(regDist) | ||
|
|
||
| exposedFaces = 2 | ||
| if self.props['inhibitedEnds'].getValue() == 'Top' or self.props['inhibitedEnds'].getValue() == 'Bottom': | ||
| if self.props['inhibitedEnds'].getValue() == InhibitedEnds.TOP or self.props['inhibitedEnds'].getValue() == InhibitedEnds.BOTTOM: | ||
| exposedFaces = 1 | ||
| if self.props['inhibitedEnds'].getValue() == 'Both': | ||
| if self.props['inhibitedEnds'].getValue() == InhibitedEnds.BOTH: | ||
| exposedFaces = 0 | ||
|
|
||
| return coreArea + (exposedFaces * faceArea) | ||
|
|
@@ -172,7 +179,7 @@ def getMassFlux(self, massIn, dTime, regDist, dRegDist, position, density): | |
| # If a position in the grain is queried, the mass flow is the input mass, from the top face, | ||
| # and from the tube up to the point. The diameter is the core. | ||
| if position <= endPos[1]: | ||
| if self.props['inhibitedEnds'].getValue() in ('Top', 'Both'): | ||
| if self.props['inhibitedEnds'].getValue() in (InhibitedEnds.TOP, InhibitedEnds.BOTH): | ||
| top = 0 | ||
| countedCoreLength = position | ||
| else: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should try to take this opportunity to make a migration that converts "top"->"forward" and "bottom"->"aft".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can see this would only be present on the
.ricfiles. Is this assumption correct?So that the necessary migration can be done I don't want to miss anything.
Any chance for a more complex
.ricfile to be made available so I can use it for testing too?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay! Here's a kind of silly example that uses all 4 inhibitor configurations:

example.ric.txt
and what it looks like on my end, so you can verify nothing changed: