Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Mark units that are aliases, and use alias string for uString comparisons so sameUnits will allow plurals, etc. (#1193) #1194

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Mark units that are aliases, and use alias string for uString comparisons so sameUnits will allow plurals, etc. (#1193) #1194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion lib/Units.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,11 +786,19 @@ our %known_units = (
},
);

# A map from unit name to its aliases
our %unit_aliases;

# A map from units name to what it is an alias for
our %unit_aliased_to;

# Process aliases.
for my $unit (keys %known_units) {
if (ref $known_units{$unit}{aliases} eq 'ARRAY') {
my $aliases = delete $known_units{$unit}{aliases};
$known_units{$_} = $known_units{$unit} for @$aliases;
$known_units{$_} = $known_units{$unit} for @$aliases;
$unit_aliased_to{$_} = $unit for @$aliases;
$unit_aliases{$unit} = $aliases;
}
}

Expand Down
22 changes: 14 additions & 8 deletions macros/contexts/contextUnits.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,11 @@ package context::Units::Context;
# The units from the original Units package
#
our %UNITS = (%Units::known_units);
$UNITS{$_} = $UNITS{L} for ('litre', 'litres', 'litre', 'litres'); # add these extras
our %ALIAS = (%Units::unit_aliased_to);
for ('litre', 'litres', 'liter', 'liters') {
$UNITS{$_} = $UNITS{L};
$ALIAS{$_} = 'L';
}
Comment on lines +689 to +692
Copy link
Member

Choose a reason for hiding this comment

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

Why are these extras added here instead of to the Units.pm file? I am curious as to why this is done differently for liters than for all other units that use the aliases key in Units.pm.

Copy link
Member Author

@dpvc dpvc Feb 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I had originally not wanted to mess with the Units.pm file, so I added them here, mostly as an illustration of how to do that. You can't use aliases here, as the Units.pm file has already processed and removed the aliases properties, so we have to do it by hand here after the fact.

I think the Units.pm file does need a lot of work in terms of aliases, but that wasn't my priority here.

Copy link
Member

Choose a reason for hiding this comment

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

I see.

I didn't mean to say that you should handle the work on aliases in this pull request. I just wanted to point that out, and that we need to work on it.

Interestingly, I noticed that I get somewhat different behavior from trying to use grams with the parserNumberWithUnits.pl versus with the new contextUnits.pl macro. With the parserNumberWithUnits.pl macro the message is 'gra' is not defined in this context, while with the contextUnits.pl macro the message is 'rams' is not defined in this context. The legacy macro sees ms as a unit and has a problem with gra, but the new macro sees g as a unit and has a problem with rams.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the two worked very differently about how they determined the units. The old units context tried to remove units from the end of the student answer and parse the rest as a formula, so it was essentially right-to-left handling of the units. But the new contest integrates units into the parsing as actual MathObjects, so it is done left-to-right, as with all the parsing. When there is a run of characters like "grams", MathObjects will take the maximal substring that matches one of its defined values (constants, functions, etc.), so in this case, since "gr", "gra", "gram", and "grams" are not defined in Units.pm, the g is the longest match.

The units context currently defines squared and square so that you can do square meters or seconds squared. I suppose one could add milli, centi, deci, etc., so that you could use millimeters, decilitres and so on without having to add all the separate aliases.


#
# The categories of units that can be selected.
Expand Down Expand Up @@ -782,6 +786,7 @@ sub addUnit {
isConstant => 1
}
);
$constants->set($name => { ustring => $ALIAS{$name} }) if $ALIAS{$name};
$constants->add(map { $_ => { alias => $name } } @$aliases) if $aliases;
$self->addUnitAliases($name) unless $options{noaliases};
} else {
Expand Down Expand Up @@ -1326,25 +1331,26 @@ sub fString {
#
# Get the string version using original units using:
# The original order and powers if $exact is set, or
# Alphabetic order and fractions otherwise.
# Alphabetic order and fractions otherwise. Use
# alias strings to normalize the result.
#
sub uString {
my ($self, $exact) = @_;
return $self->stringFor('nunits', 'dunits', $exact ? $self->{order} : undef, !$exact);
return $self->stringFor('nunits', 'dunits', $exact ? $self->{order} : undef, !$exact, 0, 1);
}

#
# Creates the string version using the given order and power settings
#
sub stringFor {
my ($self, $key1, $key2, $order, $noNegativePowers, $allowEmptyNumerator) = @_;
my ($self, $key1, $key2, $order, $noNegativePowers, $allowEmptyNumerator, $useAlias) = @_;
my ($nunits, $dunits) = ({ %{ $self->{$key1} } }, { %{ $self->{$key2} } });
$order = [ main::lex_sort(keys %$nunits, keys %$dunits) ] unless $order;
my ($ns, $ds) = ([], []);
my $constants = $self->context->constants;
for my $u (@$order) {
$self->pushUnitString($ns, $ds, $u, $nunits->{$u}, $noNegativePowers);
$self->pushUnitString($ds, $ns, $u, $dunits->{$u}, $noNegativePowers);
$self->pushUnitString($ns, $ds, $u, $nunits->{$u}, $noNegativePowers, $useAlias);
$self->pushUnitString($ds, $ns, $u, $dunits->{$u}, $noNegativePowers, $useAlias);
$nunits->{$u} = $dunits->{$u} = 0; # don't include them again
}
my ($num, $den) = (join(' ', @$ns), join(' ', @$ds));
Expand All @@ -1359,10 +1365,10 @@ sub stringFor {
# a negative power or not
#
sub pushUnitString {
my ($self, $units, $invert, $u, $n, $noNegativePowers) = @_;
my ($self, $units, $invert, $u, $n, $noNegativePowers, $useAlias) = @_;
return unless $n;
my $def = $self->context->constants->get($u);
my $unit = ($def->{string} || $u);
my $unit = (($useAlias ? $def->{ustring} : '') || $def->{string} || $u);
if (!$noNegativePowers && ($self->{negativePowers}{$u} || $self->getFlag('useNegativePowers'))) {
push(@$invert, $unit . "^-$n");
} else {
Expand Down