Skip to content

Commit 7e75830

Browse files
authored
Fix part of #4044: Add protos & testing library for polynomials (#4050)
## Explanation Fix part of #4044 Originally copied from #2173 when it was in proof-of-concept form This PR introduces the protos and test subject to represent polynomials. For the purposes of upcoming classifier work, a polynomial is defined as a sum of terms where each term has a coefficient and zero or more variables with positive integer powers. This representation provides support for all real polynomials, and keeps a nicely structured representation for coefficients that actually allows for retaining integers and rational values (this will become more evident when math expression -> polynomial conversion is added). Furthermore, various extensions files are added to help support some of the fundamental operations (mainly needed by test subjects). These operations are being thoroughly tested, and will be augmented with a lot more functionality in upcoming PRs. The new polynomial test subject doesn't have new tests to keep this PR focused on production code, and since it's relatively easy to verify as correct via review. #4100 is tracking adding tests. This structure will be utilized in a later PR in IsEquivalentTo classifier implementations for each numeric expression, algebraic expression, and math equation interaction. For specific details on this classifier, see [the PRD](https://docs.google.com/document/d/1x2vcSjocJUXkwwlce5Gjjq_Z83ykVIn2Fp0BcnrOrTg/edit#heading=h.1q3av9yssyi5). Polynomials are the ideal structure for verifying equivalence since they fully collapse expressions regardless of associativity, commutativity, and distributivity (to some extent--caveats will be noted in the future PR that converts expressions to this new polynomial structure). Slightly separate from polynomials, ``FloatExtensions`` was updated to include better epsilon values for comparing both floats and doubles (and a separate one is used for doubles). These are loosely based on the computed machine epsilon value listed here: https://en.wikipedia.org/wiki/Machine_epsilon, but it uses a higher order of magnitude and rounding for a bit more "wiggle room" for equality (so that it's not essentially replicating '==' for values close to 1). ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only N/A -- proto & testing library only change, and the proto changes are only additions. No UI functionality is yet affected by these changes. Commit history: * Copy proto-based changes from #2173. * Introduce math.proto & refactor math extensions. Much of this is copied from #2173. * Migrate tests & remove unneeded prefix. * Add needed newline. * Some needed Fraction changes. * Introduce math expression + equation protos. Also adds testing libraries for both + fractions & reals (new structure). Most of this is copied from #2173. * Add protos + testing lib for commutative exprs. * Add protos & test libs for polynomials. * Lint fix. * Lint fixes. * Fix broken test post-refactor. * Post-merge fix. * Add regex check, docs, and resolve TODOs. This also changes regex handling in the check to be more generic for better flexibility when matching files. * Lint fix. * Fix failing static checks. * Fix broken CI checks. Adds missing KDocs, test file exemptions, and fixes the Gradle build. * Lint fixes. * Add docs & exempted tests. * Remove blank line. * Add docs + tests. * Address reviewer comments + other stuff. This also fixes a typo and incorrectly ordered exemptions list I noticed during development of downstream PRs. * Move StringExtensions & fraction parsing. This splits fraction parsing between UI & utility components. * Address reviewer comments. * Alphabetize test exemptions. * Add missing KDocs. * Remove the ComparableOperationList wrapper. * Use more intentional epsilons for float comparing. * Remove failing test. * Fix broken build. * Fix broken build post-merge. * Post-merge fix. * More post-merge fixes.
1 parent 4805c65 commit 7e75830

File tree

18 files changed

+2027
-40
lines changed

18 files changed

+2027
-40
lines changed

domain/src/main/java/org/oppia/android/domain/util/InteractionObjectExtensions.kt

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package org.oppia.android.domain.util
22

33
import org.oppia.android.app.model.ClickOnImage
4-
import org.oppia.android.app.model.Fraction
54
import org.oppia.android.app.model.ImageWithRegions
65
import org.oppia.android.app.model.InteractionObject
76
import org.oppia.android.app.model.InteractionObject.ObjectTypeCase.BOOL_VALUE
@@ -107,15 +106,6 @@ private fun ImageWithRegions.toAnswerString(): String =
107106
private fun ClickOnImage.toAnswerString(): String =
108107
"[(${clickedRegionsList.joinToString()}), (${clickPosition.x}, ${clickPosition.y})]"
109108

110-
// https://github.com/oppia/oppia/blob/37285a/core/templates/dev/head/domain/objects/FractionObjectFactory.ts#L47
111-
private fun Fraction.toAnswerString(): String {
112-
val fractionString = if (numerator != 0) "$numerator/$denominator" else ""
113-
val mixedString = if (wholeNumber != 0) "$wholeNumber $fractionString" else ""
114-
val positiveFractionString = if (mixedString.isNotEmpty()) mixedString else fractionString
115-
val negativeString = if (isNegative) "-" else ""
116-
return if (positiveFractionString.isNotEmpty()) "$negativeString$positiveFractionString" else "0"
117-
}
118-
119109
private fun TranslatableHtmlContentId.toAnswerString(): String {
120110
return "content_id=$contentId"
121111
}

domain/src/test/java/org/oppia/android/domain/classify/rules/numericinput/NumericInputEqualsRuleClassifierProviderTest.kt

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import org.junit.runner.RunWith
1212
import org.oppia.android.app.model.WrittenTranslationContext
1313
import org.oppia.android.domain.classify.InteractionObjectTestBuilder
1414
import org.oppia.android.testing.assertThrows
15-
import org.oppia.android.util.math.FLOAT_EQUALITY_INTERVAL
15+
import org.oppia.android.util.math.DOUBLE_EQUALITY_EPSILON
1616
import org.robolectric.annotation.Config
1717
import org.robolectric.annotation.LooperMode
1818
import javax.inject.Inject
@@ -33,13 +33,11 @@ class NumericInputEqualsRuleClassifierProviderTest {
3333
private val NEGATIVE_REAL_VALUE_3_5 =
3434
InteractionObjectTestBuilder.createReal(value = -3.5)
3535
private val FIVE_TIMES_FLOAT_EQUALITY_INTERVAL =
36-
InteractionObjectTestBuilder.createReal(value = 5 * FLOAT_EQUALITY_INTERVAL)
37-
private val SIX_TIMES_FLOAT_EQUALITY_INTERVAL =
38-
InteractionObjectTestBuilder.createReal(value = 6 * FLOAT_EQUALITY_INTERVAL)
36+
InteractionObjectTestBuilder.createReal(value = 5 * DOUBLE_EQUALITY_EPSILON)
3937
private val FIVE_POINT_ONE_TIMES_FLOAT_EQUALITY_INTERVAL =
4038
InteractionObjectTestBuilder.createReal(
41-
value = 5 * FLOAT_EQUALITY_INTERVAL +
42-
FLOAT_EQUALITY_INTERVAL / 10
39+
value = 5 * DOUBLE_EQUALITY_EPSILON +
40+
DOUBLE_EQUALITY_EPSILON / 10
4341
)
4442
private val STRING_VALUE =
4543
InteractionObjectTestBuilder.createString(value = "test")
@@ -142,21 +140,6 @@ class NumericInputEqualsRuleClassifierProviderTest {
142140
assertThat(matches).isFalse()
143141
}
144142

145-
@Test
146-
fun testPositiveRealAnswer_positiveRealInput_valueAtRange_valuesDoNotMatch() {
147-
val inputs = mapOf(
148-
"x" to FIVE_TIMES_FLOAT_EQUALITY_INTERVAL
149-
)
150-
151-
val matches = inputEqualsRuleClassifier.matches(
152-
answer = SIX_TIMES_FLOAT_EQUALITY_INTERVAL,
153-
inputs = inputs,
154-
writtenTranslationContext = WrittenTranslationContext.getDefaultInstance()
155-
)
156-
157-
assertThat(matches).isFalse()
158-
}
159-
160143
@Test
161144
fun testRealAnswer_missingInput_throwsException() {
162145
val inputs = mapOf("y" to POSITIVE_REAL_VALUE_1_5)

model/src/main/proto/math.proto

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,29 @@ message ComparableOperation {
283283
}
284284
}
285285
}
286+
287+
// Represents a polynomial, e.g.: 2x^3+3x-y+7.
288+
message Polynomial {
289+
// The list of terms in this polynomial, e.g. the '2x^3', '3x', '-y', and '-7' in 2x^3+3x-y+7.
290+
repeated Term term = 1;
291+
292+
// Represents a polynomial term, i.e. a real coefficient multiplied by one or more variables, each
293+
// of which may have a power >= 1.
294+
message Term {
295+
// The coefficient of this term (which may be zero or negative), e.g. '2' in '2x^3'.
296+
Real coefficient = 1;
297+
298+
// The variables of this term. This list can be zero or more variables long (where zero
299+
// variables indicate a constant polynomial term).
300+
repeated Variable variable = 2;
301+
302+
// A variable within the term, that is, a variable with a positive power.
303+
message Variable {
304+
// The name of the variable, e.g. 'x' in 'x^3'.
305+
string name = 1;
306+
307+
// The power of the variable, e.g. '3' in 'x^3'.
308+
uint32 power = 2;
309+
}
310+
}
311+
}

scripts/assets/test_file_exemptions.textproto

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -650,8 +650,9 @@ exempted_file_path: "testing/src/main/java/org/oppia/android/testing/espresso/Ko
650650
exempted_file_path: "testing/src/main/java/org/oppia/android/testing/junit/DefineAppLanguageLocaleContext.kt"
651651
exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/ComparableOperationSubject.kt"
652652
exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/FractionSubject.kt"
653-
exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/MathEquationSubject.kt"
654653
exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/MathExpressionSubject.kt"
654+
exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/MathEquationSubject.kt"
655+
exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/PolynomialSubject.kt"
655656
exempted_file_path: "testing/src/main/java/org/oppia/android/testing/math/RealSubject.kt"
656657
exempted_file_path: "testing/src/main/java/org/oppia/android/testing/mockito/MockitoKotlinHelper.kt"
657658
exempted_file_path: "testing/src/main/java/org/oppia/android/testing/network/ApiMockLoader.kt"

testing/src/main/java/org/oppia/android/testing/math/BUILD.bazel

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,24 @@ kt_android_library(
7474
],
7575
)
7676

77+
kt_android_library(
78+
name = "polynomial_subject",
79+
testonly = True,
80+
srcs = [
81+
"PolynomialSubject.kt",
82+
],
83+
visibility = [
84+
"//:oppia_testing_visibility",
85+
],
86+
deps = [
87+
":real_subject",
88+
"//model/src/main/proto:math_java_proto_lite",
89+
"//third_party:com_google_truth_extensions_truth-liteproto-extension",
90+
"//third_party:com_google_truth_truth",
91+
"//utility/src/main/java/org/oppia/android/util/math:extensions",
92+
],
93+
)
94+
7795
kt_android_library(
7896
name = "real_subject",
7997
testonly = True,
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
package org.oppia.android.testing.math
2+
3+
import com.google.common.truth.FailureMetadata
4+
import com.google.common.truth.IntegerSubject
5+
import com.google.common.truth.StringSubject
6+
import com.google.common.truth.Truth.assertAbout
7+
import com.google.common.truth.Truth.assertThat
8+
import com.google.common.truth.Truth.assertWithMessage
9+
import com.google.common.truth.extensions.proto.LiteProtoSubject
10+
import org.oppia.android.app.model.Polynomial
11+
import org.oppia.android.testing.math.PolynomialSubject.Companion.assertThat
12+
import org.oppia.android.testing.math.RealSubject.Companion.assertThat
13+
import org.oppia.android.util.math.getConstant
14+
import org.oppia.android.util.math.isConstant
15+
import org.oppia.android.util.math.toPlainText
16+
17+
// TODO(#4100): Add tests for this class.
18+
19+
/**
20+
* Truth subject for verifying properties of [Polynomial]s.
21+
*
22+
* Note that this class is also a [LiteProtoSubject] so other aspects of the underlying [Polynomial]
23+
* proto can be verified through inherited methods.
24+
*
25+
* Call [assertThat] to create the subject.
26+
*/
27+
class PolynomialSubject(
28+
metadata: FailureMetadata,
29+
private val actual: Polynomial?
30+
) : LiteProtoSubject(metadata, actual) {
31+
private val nonNullActual by lazy {
32+
checkNotNull(actual) {
33+
"Expected polynomial to be defined, not null (is the expression/equation not a valid" +
34+
" polynomial?)"
35+
}
36+
}
37+
38+
/** Verifies that the represented [Polynomial] is null (i.e. not a valid polynomial). */
39+
fun isNotValidPolynomial() {
40+
assertWithMessage(
41+
"Expected polynomial to be undefined, but was: ${actual?.toPlainText()}"
42+
).that(actual).isNull()
43+
}
44+
45+
/**
46+
* Verifies that the represented [Polynomial] is a constant (i.e. [Polynomial.isConstant] and
47+
* returns a [RealSubject] to verify the value of the constant polynomial.
48+
*/
49+
fun isConstantThat(): RealSubject {
50+
assertWithMessage("Expected polynomial to be constant, but was: ${nonNullActual.toPlainText()}")
51+
.that(nonNullActual.isConstant())
52+
.isTrue()
53+
return assertThat(nonNullActual.getConstant())
54+
}
55+
56+
/**
57+
* Returns an [IntegerSubject] to test [Polynomial.getTermCount].
58+
*
59+
* This method never fails since the underlying property defaults to 0 if there are no terms
60+
* defined in the polynomial (unless the polynomial is null).
61+
*/
62+
fun hasTermCountThat(): IntegerSubject = assertThat(nonNullActual.termCount)
63+
64+
/**
65+
* Returns a [PolynomialTermSubject] to test [Polynomial.getTerm] for the specified index.
66+
*
67+
* This method throws if the index doesn't correspond to a valid term. Callers should first verify
68+
* the term count using [hasTermCountThat].
69+
*/
70+
fun term(index: Int): PolynomialTermSubject = assertThat(nonNullActual.termList[index])
71+
72+
/**
73+
* Returns a [StringSubject] to test the plain-text representation of the [Polynomial] (i.e. via
74+
* [Polynomial.toPlainText]).
75+
*/
76+
fun evaluatesToPlainTextThat(): StringSubject = assertThat(nonNullActual.toPlainText())
77+
78+
companion object {
79+
/** Returns a new [PolynomialSubject] to verify aspects of the specified [Polynomial] value. */
80+
fun assertThat(actual: Polynomial?): PolynomialSubject =
81+
assertAbout(::PolynomialSubject).that(actual)
82+
83+
private fun assertThat(actual: Polynomial.Term): PolynomialTermSubject =
84+
assertAbout(::PolynomialTermSubject).that(actual)
85+
86+
private fun assertThat(actual: Polynomial.Term.Variable): PolynomialTermVariableSubject =
87+
assertAbout(::PolynomialTermVariableSubject).that(actual)
88+
}
89+
90+
/**
91+
* Truth subject for verifying properties of [Polynomial.Term]s.
92+
*
93+
* Note that this class is also a [LiteProtoSubject] so other aspects of the underlying
94+
* [Polynomial.Term] proto can be verified through inherited methods.
95+
*/
96+
class PolynomialTermSubject(
97+
metadata: FailureMetadata,
98+
private val actual: Polynomial.Term
99+
) : LiteProtoSubject(metadata, actual) {
100+
/**
101+
* Returns a [RealSubject] to test [Polynomial.Term.getCoefficient] for the represented term.
102+
*
103+
* This method never fails since the underlying property defaults to a default instance if it's
104+
* not defined in the term.
105+
*/
106+
fun hasCoefficientThat(): RealSubject = assertThat(actual.coefficient)
107+
108+
/**
109+
* Returns an [IntegerSubject] to test [Polynomial.Term.getVariableCount] for the represented
110+
* term.
111+
*
112+
* This method never fails since the underlying property defaults to 0 if there are no variables
113+
* in the represented term.
114+
*/
115+
fun hasVariableCountThat(): IntegerSubject = assertThat(actual.variableCount)
116+
117+
/**
118+
* Returns a [PolynomialTermVariableSubject] to test [Polynomial.Term.getVariable] for the
119+
* specified index.
120+
*
121+
* This method throws if the index doesn't correspond to a valid variable. Callers should first
122+
* verify the variable count using [hasVariableCountThat].
123+
*/
124+
fun variable(index: Int): PolynomialTermVariableSubject =
125+
assertThat(actual.variableList[index])
126+
}
127+
128+
/**
129+
* Truth subject for verifying properties of [Polynomial.Term.Variable]s.
130+
*
131+
* Note that this class is also a [LiteProtoSubject] so other aspects of the underlying
132+
* [Polynomial.Term.Variable] proto can be verified through inherited methods.
133+
*/
134+
class PolynomialTermVariableSubject(
135+
metadata: FailureMetadata,
136+
private val actual: Polynomial.Term.Variable
137+
) : LiteProtoSubject(metadata, actual) {
138+
/**
139+
* Returns a [StringSubject] to test [Polynomial.Term.Variable.getName] for the represented
140+
* variable.
141+
*
142+
* This method never fails since the underlying property defaults to empty string if it's not
143+
* defined in the variable.
144+
*/
145+
fun hasNameThat(): StringSubject = assertThat(actual.name)
146+
147+
/**
148+
* Returns an [IntegerSubject] to test [Polynomial.Term.Variable.getPower] for the represented
149+
* variable.
150+
*
151+
* This method never fails since the underlying property defaults to 0 if it's not defined in
152+
* the variable.
153+
*/
154+
fun hasPowerThat(): IntegerSubject = assertThat(actual.power)
155+
}
156+
}

utility/src/main/java/org/oppia/android/util/math/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ kt_android_library(
99
srcs = [
1010
"FloatExtensions.kt",
1111
"FractionExtensions.kt",
12+
"PolynomialExtensions.kt",
1213
"RatioExtensions.kt",
14+
"RealExtensions.kt",
1315
],
1416
visibility = [
1517
"//:oppia_api_visibility",

utility/src/main/java/org/oppia/android/util/math/FloatExtensions.kt

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,40 @@ package org.oppia.android.util.math
22

33
import kotlin.math.abs
44

5-
/** The error margin used for float equality by [Float.approximatelyEquals]. */
6-
const val FLOAT_EQUALITY_INTERVAL = 1e-5
5+
/**
6+
* The error margin used for approximately [Float] equality checking.
7+
*
8+
* Note that the machine epsilon value from https://en.wikipedia.org/wiki/Machine_epsilon is defined
9+
* defined as the smallest value that, when added to, or subtract from, 1, will result in a value
10+
* that is exactly equal to 1. A slightly larger value is picked here for some allowance in
11+
* variance.
12+
*/
13+
const val FLOAT_EQUALITY_EPSILON: Float = 1e-6f
714

8-
/** Returns whether this float approximately equals another based on a consistent epsilon value. */
15+
/**
16+
* The error margin used for approximately [Double] equality checking.
17+
*
18+
* See [FLOAT_EQUALITY_EPSILON] for an explanation of this value.
19+
*/
20+
const val DOUBLE_EQUALITY_EPSILON: Double = 1e-15
21+
22+
/**
23+
* Returns whether this float approximately equals another based on a consistent epsilon value
24+
* ([FLOAT_EQUALITY_EPSILON]).
25+
*/
926
fun Float.approximatelyEquals(other: Float): Boolean {
10-
return abs(this - other) < FLOAT_EQUALITY_INTERVAL
27+
return abs(this - other) < FLOAT_EQUALITY_EPSILON
1128
}
1229

13-
/** Returns whether this double approximately equals another based on a consistent epsilon value. */
30+
/** Returns whether this double approximately equals another based on a consistent epsilon value
31+
* ([DOUBLE_EQUALITY_EPSILON]).
32+
*/
1433
fun Double.approximatelyEquals(other: Double): Boolean {
15-
return abs(this - other) < FLOAT_EQUALITY_INTERVAL
34+
return abs(this - other) < DOUBLE_EQUALITY_EPSILON
1635
}
36+
37+
/**
38+
* Returns a string representation of this [Double] that keeps the double in pure decimal and never
39+
* relies on scientific notation (unlike [Double.toString]).
40+
*/
41+
fun Double.toPlainString(): String = toBigDecimal().toPlainString()

0 commit comments

Comments
 (0)