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

Fix negative unaries - Take 2 #318

Merged
merged 3 commits into from
Apr 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ dev = [
"ruff",
"black",
"debugpy",
"antlr4-tools",
]

[project.scripts]
Expand Down
1 change: 1 addition & 0 deletions src/atopile/cli/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ def generate_assertion_report(build_ctx: BuildContext) -> None:
"""Generate a report based on assertions made in the source code."""
atopile.assertions.generate_assertion_report(build_ctx)


@muster.register("view-dict")
def generate_view_dict(build_ctx: BuildContext) -> None:
"""Generate a dictionary for the viewer."""
Expand Down
6 changes: 4 additions & 2 deletions src/atopile/expressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def _custom_float_format(value, max_decimals: int):

def _best_units(qty_a: pint.Quantity, qty_b: pint.Quantity) -> PlainUnit:
"""Return the best unit for the two quantities."""
if str(qty_a.to(qty_b.units).magnitude) < str(qty_b.to(qty_a.units).magnitude):
if len(str(qty_a.to(qty_b.units).magnitude)) > len(str(qty_b.to(qty_a.units).magnitude)):
return qty_a.units
return qty_b.units

Expand Down Expand Up @@ -303,7 +303,9 @@ def __eq__(self, other: object) -> bool:
# NOTE: realistically this is only useful for testing
if isinstance(other, RangedValue):
return self.min_qty == other.min_qty and self.max_qty == other.max_qty
if self.min_val == self.max_val == other and self.unit.dimensionless:

# NOTE: this doesn't work for farenheit or kelvin, but everything else is okay
if self.min_val == self.max_val == other and (self.unit.dimensionless or other == 0):
return True
return False

Expand Down
150 changes: 74 additions & 76 deletions src/atopile/front_end.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,10 @@ def __init__(
val_b: Number | pint.Quantity,
unit: Optional[pint.Unit] = None,
src_ctx: Optional[ParserRuleContext] = None,
pretty_unit: Optional[str] = None,
):
self.src_ctx = src_ctx
super().__init__(val_a, val_b, unit)
super().__init__(val_a, val_b, unit, pretty_unit)


class Expression(expressions.Expression):
Expand Down Expand Up @@ -379,19 +380,23 @@ def visitBoolean_(self, ctx: ap.Boolean_Context) -> bool:

def visitLiteral_physical(self, ctx: ap.Literal_physicalContext) -> RangedValue:
"""Yield a physical value from a physical context."""
if ctx.implicit_quantity():
return self.visitImplicit_quantity(ctx.implicit_quantity())
if ctx.quantity():
return self.visitQuantity(ctx.quantity())
if ctx.bilateral_quantity():
return self.visitBilateral_quantity(ctx.bilateral_quantity())
if ctx.bound_quantity():
return self.visitBound_quantity(ctx.bound_quantity())

raise ValueError # this should be protected because it shouldn't be parseable

def visitImplicit_quantity(self, ctx: ap.Implicit_quantityContext) -> RangedValue:
def visitQuantity(self, ctx: ap.QuantityContext) -> RangedValue:
"""Yield a physical value from an implicit quantity context."""
value = float(ctx.NUMBER().getText())

# Ignore the positive unary operator
if ctx.MINUS():
value = -value

if ctx.name():
unit = _get_unit_from_ctx(ctx.name())
else:
Expand All @@ -406,26 +411,21 @@ def visitImplicit_quantity(self, ctx: ap.Implicit_quantityContext) -> RangedValu

def visitBilateral_quantity(self, ctx: ap.Bilateral_quantityContext) -> RangedValue:
"""Yield a physical value from a bilateral quantity context."""
nominal = float(ctx.bilateral_nominal().NUMBER().getText())

if ctx.bilateral_nominal().name():
unit = _get_unit_from_ctx(ctx.bilateral_nominal().name())
else:
unit = pint.Unit("")
nominal_quantity = self.visitQuantity(ctx.quantity())

tol_ctx: ap.Bilateral_toleranceContext = ctx.bilateral_tolerance()
tol_num = float(tol_ctx.NUMBER().getText())

# Handle proportional tolerances
if tol_ctx.PERCENT():
tol_divider = 100
# FIXME: hardcoding this seems wrong, but the parser/lexer wasn't picking up on it
elif tol_ctx.name() and tol_ctx.name().getText() == "ppm":
tol_divider = 1e6
else:
tol_divider = None

if tol_divider:
if nominal == 0:
if nominal_quantity == 0:
raise errors.AtoError.from_ctx(
tol_ctx,
"Can't calculate tolerance percentage of a nominal value of zero",
Expand All @@ -434,79 +434,85 @@ def visitBilateral_quantity(self, ctx: ap.Bilateral_quantityContext) -> RangedVa
# In this case, life's a little easier, and we can simply multiply the nominal
return RangedValue(
src_ctx=ctx,
val_a=nominal * (1 - tol_num / tol_divider),
val_b=nominal * (1 + tol_num / tol_divider),
unit=unit,
val_a=nominal_quantity.min_val - (nominal_quantity.min_val * tol_num / tol_divider),
val_b=nominal_quantity.max_val + (nominal_quantity.max_val * tol_num / tol_divider),
unit=nominal_quantity.unit,
)

# Handle tolerances with units
if tol_ctx.name():
# In this case there's a named unit on the tolerance itself
# We need to make sure it's dimensionally compatible with the nominal
tolerance_unit = _get_unit_from_ctx(tol_ctx.name())
tol_quantity = RangedValue(-tol_num, tol_num, _get_unit_from_ctx(tol_ctx.name()), tol_ctx)

# If the nominal has no unit, then we take the unit's tolerance for the nominal
if nominal_quantity.unit == pint.Unit(""):
return RangedValue(
src_ctx=ctx,
val_a=nominal_quantity.min_val - tol_quantity.min_val,
val_b=nominal_quantity.max_val + tol_quantity.max_val,
unit=tol_quantity.unit,
)

# If the nominal has a unit, then we rely on the ranged value's unit compatibility
try:
tolerance = (tol_num * tolerance_unit).to(unit).magnitude
return nominal_quantity + tol_quantity
except pint.DimensionalityError as ex:
raise errors.AtoTypeError.from_ctx(
tol_ctx.name(),
f"Tolerance unit '{tolerance_unit}' is not dimensionally"
f" compatible with nominal unit '{unit}'",
f"Tolerance unit '{tol_quantity.unit}' is not dimensionally"
f" compatible with nominal unit '{nominal_quantity.unit}'",
) from ex

return RangedValue(
src_ctx=ctx,
val_a=nominal - tolerance,
val_b=nominal + tolerance,
unit=unit,
)

# If there's no unit or percent, then we have a simple tolerance in the same units
# as the nominal
return RangedValue(
src_ctx=ctx,
val_a=nominal - tol_num,
val_b=nominal + tol_num,
unit=unit,
val_a=nominal_quantity.min_val - tol_num,
val_b=nominal_quantity.max_val + tol_num,
unit=nominal_quantity.unit,
)

def visitBound_quantity(self, ctx: ap.Bound_quantityContext) -> RangedValue:
"""Yield a physical value from a bound quantity context."""

def _parse_end(
ctx: ap.Quantity_endContext,
) -> tuple[float, Optional[pint.Unit]]:
value = float(ctx.NUMBER().getText())
if ctx.name():
unit = _get_unit_from_ctx(ctx.name())
else:
unit = None
return value, unit
start = self.visitQuantity(ctx.quantity(0))
assert start.tolerance == 0
end = self.visitQuantity(ctx.quantity(1))
assert end.tolerance == 0

start_val, start_unit = _parse_end(ctx.quantity_end(0))
end_val, end_unit = _parse_end(ctx.quantity_end(1))
# If only one of them has a unit, take the unit from the one which does
if (start.unit == pint.Unit("")) ^ (end.unit == pint.Unit("")):
if start.unit == pint.Unit(""):
known_unit = end.unit
known_pretty_unit = end.pretty_unit
else:
known_unit = start.unit
known_pretty_unit = start.pretty_unit

if start_unit is None and end_unit is None:
unit = pint.Unit("")
elif start_unit and end_unit:
unit = start_unit
try:
end_val = (end_val * end_unit).to(start_unit).magnitude
except pint.DimensionalityError as ex:
raise errors.AtoTypeError.from_ctx(
ctx,
f"Tolerance unit '{end_unit}' is not dimensionally"
f" compatible with nominal unit '{start_unit}'",
) from ex
elif start_unit:
unit = start_unit
else:
unit = end_unit
return RangedValue(
src_ctx=ctx,
val_a=start.min_val,
val_b=end.min_val,
unit=known_unit,
pretty_unit=known_pretty_unit,
)

return RangedValue(
src_ctx=ctx,
val_a=start_val,
val_b=end_val,
unit=unit,
)
# If they've both got units, let the RangedValue handle
# the dimensional compatibility
try:
return RangedValue(
src_ctx=ctx,
val_a=start.min_qty,
val_b=end.min_qty,
pretty_unit=start.pretty_unit,
)
except pint.DimensionalityError as ex:
raise errors.AtoTypeError.from_ctx(
ctx,
f"Tolerance unit '{end.unit}' is not dimensionally"
f" compatible with nominal unit '{start.unit}'",
) from ex


class HandleStmtsFunctional(AtopileParserVisitor):
Expand Down Expand Up @@ -625,35 +631,27 @@ def visitTerm(self, ctx: ap.TermContext) -> expressions.NumericishTypes:
return expressions.defer_operation_factory(
self.visit(ctx.term()),
operator.mul,
self.visit(ctx.factor()),
self.visit(ctx.power()),
)

if ctx.DIV():
return expressions.defer_operation_factory(
self.visit(ctx.term()),
operator.truediv,
self.visit(ctx.factor()),
self.visit(ctx.power()),
)

return self.visit(ctx.factor())

def visitFactor(self, ctx: ap.FactorContext) -> expressions.NumericishTypes:
# Ignore the unary plus operator

if ctx.MINUS():
return operator.neg(self.visit(ctx.power()))

return self.visit(ctx.power())

def visitPower(self, ctx: ap.PowerContext) -> expressions.NumericishTypes:
if ctx.factor():
if ctx.POWER():
return expressions.defer_operation_factory(
self.visit(ctx.atom()),
self.visit(ctx.atom(0)),
operator.pow,
self.visit(ctx.factor()),
self.visit(ctx.atom(1)),
)

return self.visit(ctx.atom())
return self.visit(ctx.atom(0))

def visitAtom(self, ctx: ap.AtomContext) -> expressions.NumericishTypes:
if ctx.arithmetic_group():
Expand Down
21 changes: 7 additions & 14 deletions src/atopile/parser/AtopileParser.g4
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,12 @@ arithmetic_expression
;

term
: term ('*' | '/') factor
| factor
: term ('*' | '/') power
| power
;

factor
Copy link
Contributor Author

@mawildoer mawildoer Apr 26, 2024

Choose a reason for hiding this comment

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

This will mean that you can't put a unary in front of brackets or variables

: '+' factor
| '-' factor
| power;

power
: atom ('**' factor)?
: atom ('**' atom)?
;


Expand All @@ -112,13 +107,11 @@ arithmetic_group
literal_physical
: bound_quantity
| bilateral_quantity
| implicit_quantity;
| quantity;

bound_quantity: quantity_end 'to' quantity_end;
quantity_end: NUMBER name?;
bilateral_quantity: bilateral_nominal PLUS_OR_MINUS bilateral_tolerance;
implicit_quantity: NUMBER name?;
bilateral_nominal: NUMBER name?;
bound_quantity: quantity 'to' quantity;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want to make tolerances (especially the to operator) part of the nesting arithmetic expressions rather than a part of an atom

bilateral_quantity: quantity PLUS_OR_MINUS bilateral_tolerance;
quantity: ('+' | '-')? NUMBER name?;
bilateral_tolerance: NUMBER ('%' | name)?;

name_or_attr: attr | name;
Expand Down