diff --git a/vcell-math/src/main/java/cbit/vcell/units/InternalUnitDefinition.java b/vcell-math/src/main/java/cbit/vcell/units/InternalUnitDefinition.java index 40bed24476..1c19bf9aef 100644 --- a/vcell-math/src/main/java/cbit/vcell/units/InternalUnitDefinition.java +++ b/vcell-math/src/main/java/cbit/vcell/units/InternalUnitDefinition.java @@ -43,8 +43,6 @@ class InternalUnitDefinition implements Matchable, Serializable { public static final String TBD_SYMBOL = "tbd"; - private static final java.text.NumberFormat numberFormatForRounding = - new java.text.DecimalFormat("#0.0#E0#"); //VC standard Unit definitions public static final InternalUnitDefinition UNIT_s; @@ -110,8 +108,6 @@ class InternalUnitDefinition implements Matchable, Serializable { //private static ucar.units_vcell.PrefixDB prefixDB = null; static { - numberFormatForRounding.setMaximumFractionDigits(12); - defs = new ArrayList(); InternalUnitDefinition unit_s = null; @@ -674,19 +670,20 @@ public InternalUnitDefinition raiseTo(RationalNumber exp) { /** - * Insert the method's description here. - * Creation date: (6/13/2004 3:03:36 PM) - * @return double - * @param value double + * Round a value to at most 12 significant digits. + * + * Implementation note: previously used a shared static DecimalFormat + * for round-trip format/parse. DecimalFormat is not thread-safe and the + * shared instance produced corrupted strings (e.g., "11E.11-83E-83") + * under concurrent access from background-thread unit arithmetic, + * causing NumberFormatException to escape the catch block. This + * implementation is stateless and locale-independent. */ public static double round(double value) { - try { - double roundedValue = numberFormatForRounding.parse(numberFormatForRounding.format(value)).doubleValue(); - return roundedValue; - }catch (java.text.ParseException e){ - e.printStackTrace(System.out); + if (Double.isNaN(value) || Double.isInfinite(value)) { return value; } + return Double.parseDouble(String.format(java.util.Locale.US, "%.12e", value)); } diff --git a/vcell-math/src/test/java/cbit/vcell/units/InternalUnitDefinitionRoundTest.java b/vcell-math/src/test/java/cbit/vcell/units/InternalUnitDefinitionRoundTest.java new file mode 100644 index 0000000000..f7a0d3db70 --- /dev/null +++ b/vcell-math/src/test/java/cbit/vcell/units/InternalUnitDefinitionRoundTest.java @@ -0,0 +1,89 @@ +package cbit.vcell.units; + +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@Tag("Fast") +public class InternalUnitDefinitionRoundTest { + + @Test + public void roundOfOrdinaryValuesIsIdempotentToTwelveDigits() { + double[] values = { + 1.0, -1.0, 0.0, 1234.5678, -1234.5678, + 1.234567890123456789, 1e-83, 1e83, Math.PI, Math.E, + }; + for (double v : values) { + double r = InternalUnitDefinition.round(v); + // Twelve significant digits should be preserved exactly. + assertEquals(r, InternalUnitDefinition.round(r), + "round must be idempotent for value " + v); + // The rounded value must match the original to 12 sig figs. + if (v != 0.0) { + double relErr = Math.abs((r - v) / v); + assertTrue(relErr < 1e-11, + "relative error too large for " + v + ": " + relErr); + } + } + } + + @Test + public void roundPreservesSpecialValues() { + assertTrue(Double.isNaN(InternalUnitDefinition.round(Double.NaN))); + assertEquals(Double.POSITIVE_INFINITY, + InternalUnitDefinition.round(Double.POSITIVE_INFINITY)); + assertEquals(Double.NEGATIVE_INFINITY, + InternalUnitDefinition.round(Double.NEGATIVE_INFINITY)); + assertEquals(0.0, InternalUnitDefinition.round(0.0)); + // round(-0.0) returns -0.0 (sign preserved); both are mathematically zero. + assertEquals(0.0, Math.abs(InternalUnitDefinition.round(-0.0))); + } + + @Test + public void roundIsThreadSafe() throws InterruptedException { + // Regression test: a previous implementation used a shared static + // DecimalFormat. Concurrent access produced strings like + // "11E.11-83E-83" (= "1E-83" formatted twice with digits + // interleaved), which then failed Double.parseDouble with + // NumberFormatException ("multiple points" or "For input string"). + int threads = 16; + int iterationsPerThread = 5_000; + ExecutorService pool = Executors.newFixedThreadPool(threads); + AtomicInteger failures = new AtomicInteger(0); + AtomicInteger badResults = new AtomicInteger(0); + try { + for (int t = 0; t < threads; t++) { + final double base = 1.0 + t * 0.0001; + pool.submit(() -> { + for (int i = 0; i < iterationsPerThread; i++) { + double value = base * Math.pow(10, (i % 100) - 50); + try { + double r = InternalUnitDefinition.round(value); + if (Double.isNaN(r) != Double.isNaN(value)) { + badResults.incrementAndGet(); + } + } catch (RuntimeException e) { + failures.incrementAndGet(); + } + } + }); + } + } finally { + pool.shutdown(); + assertTrue(pool.awaitTermination(30, TimeUnit.SECONDS), + "thread pool did not terminate in time"); + } + assertEquals(0, failures.get(), + "round() threw under concurrent access"); + assertEquals(0, badResults.get(), + "round() returned wrong-typed result under concurrent access"); + } +}