I have done all the changes you requested, please review again.
- Qian
On 11/14/23 22:08, Waldek Hebisch wrote:
On Tue, Nov 14, 2023 at 05:47:24PM +0800, Qian Yun wrote:
On 11/14/23 06:42, Waldek Hebisch wrote:
On Sun, Nov 05, 2023 at 10:28:38AM +0800, Qian Yun wrote:
I introduce a new variable "roundingError", its valued was mistakenly
added to the 'other' class, later it was added to "total" again,
causing duplication. So I change it to only add to
the 'other' named stat.
The comparison "if n >= 0.01" is wrong, since we take 2 digits
accuracy, so it should compare against 0.005, see "significantStat".
Also "roundStat" is totally useless because it rounds to the third
digit. We can rely on "FORMAT" to do the rounding.
I suspect that intent of "if n >= 0.01" is different than you think.
Namely, small statistics are likely to have significant error
so it makes sense to supress printing of info for small classes.
I other words, it make sense to have treshold which is bigger
than treshold for rounding to 0.
How about we do no rounding at all, and print 3 digits (or more, see
following) after decimal point, and let user to decide if the last
digit is to be trusted.
Well, as you noted FORMAT will do rounding to specified precision.
_If_ small values have significant error, then it makes sense to
supress printing of such values (except for total): printout
is less cluttered and user that cares about specific class will need
longer time to get meaningful result.
AFAICS original code is first computing long statistics and if non-long
statistics are desired it throws out computed result and starts again.
It would make sense to have common code which just computes desired
statistics.
I can do this improvement.
Also, before your recent changes long format skipped "other" class
when corresponding time would print as 0 (or maybe was small). Now
it is:
Time: 0.02(evaluation) + 0.00(other) = 0.03 sec
This happened before as well (when the number is less than 0.005):
Time: 0.00(O) = 0.00 sec
Maybe this happened sometime. But I obtained:
Time: 0.02(E) = 0.02 sec
from old code and result with "0.00(other)" using new code. I think
we should supress printing of "0.00" time except for total. Old code tried
to replace "0.00" by "0" but as you show it failed at least in some
cases.
One more thing: did you try to get more accurate timing? In different
context I have found useful microsecond timings: al least on Linux it
is possible to measure real time taken be a routine with microsecond
accuracy. I am not sure how much accuracy one can get from Lisp
routines, but IIRC Clozure CL reports time with microsecond resolution.
The timing accuracy is determined by $timerTicksPerSecond which is
INTERNAL-TIME-UNITS-PER-SECOND in Lisp:
GCL: 100
CLISP: 1000000 (windows: 10^7)
ECL: 1000000
SBCL: 1000000 (x86: 1000)
CMUCL: 100
CCL: 1000000 (x86: 1000)
ABCL: 1000
Well, that is resolution of the result. But accuracy may
be lower and question is if we get accuracy significantly better
than 2 digits.
So that's probably why we were printing 2 digits in the past.
(And Lisps have lower accuracy due to 32-bit limitation.)
If that's the case, then the whole rounding logic did nothing
in the GCL era.
Shall we make it configurable how many digits to be printed?
Probably. Even if accuracy is low it make sense to have somewhat
higher resolution of the result simply to avoid extra error due
to rounding. And if system can deliver better accuracy it is
silly to loose accuracy by rounding to 2 digits. OTOH people
will mostly look at time when it is long and for long times
2 digits after dot are enough.
--
You received this message because you are subscribed to the Google Groups "FriCAS -
computer algebra system" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to fricas-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/fricas-devel/84e85534-a0ef-49fd-a783-daf13407cc74%40gmail.com.
diff --git a/src/interp/g-timer.boot b/src/interp/g-timer.boot
index 2b6e2473..bd588fc7 100644
--- a/src/interp/g-timer.boot
+++ b/src/interp/g-timer.boot
@@ -41,56 +41,40 @@ makeLongStatStringByProperty _
total := 0
str := '""
otherStatTotal := GET('other, property)
+ insignificantStat := 0
for [name,class,:ab] in listofnames repeat
- name = 'other => 'iterate
cl := first LASSOC(class, listofclasses)
n := GET(name, property)
PUT(cl, classproperty, n + GET(cl, classproperty))
total := total + n
- if n >= 0.01
- then timestr := normalizeStatAndStringify n
- else
- timestr := '""
- otherStatTotal := otherStatTotal + n
- str := makeStatString(str, timestr, name, flag)
- PUT('other, property, otherStatTotal)
- if otherStatTotal > 0 then
- timestr := normalizeStatAndStringify otherStatTotal
- str := makeStatString(str, timestr, 'other, flag)
- total := total + otherStatTotal
- cl := first LASSOC('other, listofnames)
- cl := first LASSOC(cl, listofclasses)
- PUT(cl, classproperty, otherStatTotal + GET(cl, classproperty))
- if flag ~= 'long then
- total := 0
- str := '""
+ name = 'other or flag ~= 'long => 'iterate
+ if n >= 0.01 then
+ str := makeStatString(str, n, name, flag)
+ else
+ insignificantStat := insignificantStat + n
+ if flag = 'long then
+ str := makeStatString(str, otherStatTotal + insignificantStat, 'other, flag)
+ else
for [class,name,:ab] in listofclasses repeat
n := GET(name, classproperty)
- n = 0.0 or n = 0 => 'iterate
- total := total + n
- timestr := normalizeStatAndStringify n
- str := makeStatString(str,timestr,ab,flag)
+ str := makeStatString(str, n, ab, flag)
total := STRCONC(normalizeStatAndStringify total,'" ", units)
str = '"" => total
STRCONC(str, '" = ", total)
normalizeStatAndStringify t ==
FLOATP t =>
- t := roundStat t
- t = 0.0 => '"0"
+ t < 0.01 => '"0"
FORMAT(nil,'"~,2F",t)
INTEGERP t => FORMAT(nil, '"~:d", t)
STRINGIMAGE t
-roundStat t ==
- not FLOATP t => t
- (TRUNCATE (0.5 + t * 1000.0)) / 1000.0
-
makeStatString(oldstr,time,abb,flag) ==
- time = '"" => oldstr
+ time < 0.01 => oldstr
opening := (flag = 'long => '"("; '" (")
- oldstr = '"" => STRCONC(time,opening,abb,'")")
- STRCONC(oldstr,'" + ",time,opening,abb,'")")
+ timestr := normalizeStatAndStringify time
+ oldstr = '"" => STRCONC(timestr, opening, abb, '")")
+ STRCONC(oldstr, '" + ", timestr, opening, abb, '")")
peekTimedName() == IFCAR $timedNameStack
