Commit Graph

1403 Commits (1dc691160b41bb3ed3cb15de11532f4bfcbc9429)

Author SHA1 Message Date
Mike Gerwitz f42288f3a2 tamer: obj::xmlo::reader: Begin symbol table parsing
This wasn't the simplest thing to start with, but I wanted to explore
something with a higher level of complexity.  There is some boilerplate to
observe here, including:

  1. The state stitching (as I guess I'm calling it now) of SymtableState
     with XmloReaderState is all boilerplate and requires no lookahead,
     presenting an abstraction opportunity that I was holding off on
     previously (attr parsing for XIRF requires lookahead).
  2. This is simply collecting attributes into a struct.  This can be
     abstracted away in the future.
  3. Creating stub parsers to verify that generics are stitched rather than
     being tightly coupled with another state is boilerplate that maybe can
     be abstracted away after a pattern is observed in future tests.

DEV-10863
2022-03-29 11:14:47 -04:00
Mike Gerwitz f402e51d04 tamer: parse: More flexible Transition API
This does some cleanup and adds `parse::Object` for use in disambiguating
`From` for `ParseStatus`, allowing the `Transition` API to be much more
flexible in the data it accepts and automatically converts.  This allows us
to concisely provide raw output data to be wrapped, or provide `ParseStatus`
directly when more convenient.

There aren't yet examples in the docs; I'll do so once I make sure this API
is actually utilized as intended.

DEV-10863
2022-03-25 16:45:32 -04:00
Mike Gerwitz c0fa89222e tamer: obj::xmlo::ir::Dim: New enum
This replaces u8 and will be used for the new XmloReader.

Previously I wasn't sure what direction TAMER was going to go in with
regards to dimensionality, but I do not expect that higher dimensions will
be supported, and if they are, they'd very likely compile down to lower ones
and create an illusion of higher-dimensionality.

Whatever the future holds, it's not used today, and I'd rather these types
be correct.

ASG needs changing too, but one step at a time.

DEV-10863
2022-03-25 14:28:18 -04:00
Mike Gerwitz 279ddc79d7 tamer: parse::TransitionResult: Alias=>newtype
This converts the tuple type alias into a newtype, so that we may provide
our own implementations.

This differs from a previous approach that I took, which involved making
this type `Result<(S, T), (S, E)>` so that the return values composed well
with other functions.  But the reality is that this is used only by other
`ParseState`s and `Parser`, so it's unnecessary.

However, this is also an attempt to utilize the new Try and FromResidual
traits; note how the Try associated types match precisely what I was trying
to do before, though they're used as intermediate types.  I'll see how this
evolves.

DEV-10863
2022-03-25 12:28:50 -04:00
Mike Gerwitz 2e98a69d15 Revert "tamer: parse::TransitionResult: Move common Transition into Result"
This reverts commit bf5da75096.
2022-03-25 09:17:25 -04:00
Mike Gerwitz bf5da75096 tamer: parse::TransitionResult: Move common Transition into Result
This allows the Results to compose and, importantly, is compatible with
`?` without having to put in any extra effort.

This makes puts the caller in an awkward spot, so I introduced a utility
function `result_tup0_invert` for now; we'll see if that stays or evolves
differently.

DEV-10863
2022-03-24 23:48:30 -04:00
Mike Gerwitz 9d9b1f30a8 tamer: obj::xmlo::reader: Move XmloEvent to top of module
Since this is the object produced by this parser, this is likely the most
useful first thing to present as a summary of what `XmloReader` actually
does.

DEV-10863
2022-03-24 10:14:40 -04:00
Mike Gerwitz 2e3d94c3d6 tamer: obj::xmlo::reader: Simplify wip-xmlo-xir-reader flagging
This removes the flag from most of the code, which also resolves the
indentation.  Not only was it bothering me, but I don't want (a) every line
modified when the module body is hoisted and (b) `rustfmt` to reformat
everything when that happens.

This means that everything will be built, even though it's not used, when
the flag is off, but I see that as a good thing.

DEV-10863
2022-03-24 09:45:59 -04:00
Mike Gerwitz fab7b16ea0 tamer: obj::xmlo::reader: Parse package attributes
Finally we get to do some actual parsing with all of the preparatory work!

This means that we're finally ready to fully replace the old XmloReader,
provided that I'm okay with some boilerplate / lack of abstractions for
now (and I am, because all I've been doing is working on abstractions to
prepare lowering operations).

DEV-10863
2022-03-23 16:48:51 -04:00
Mike Gerwitz ad8616aaa1 tamer: xir::attr::Attr: Convert to tuple struct with public fields
This makes more sense for pattern matching.  Encapsulation of these fields
is not necessary, given that it's passed around as an owned value and its
`new` method constructs it verbatim; the individual fields are
self-validating.

DEV-10863
2022-03-23 16:41:28 -04:00
Mike Gerwitz fbf786086a tamer: parse::Parser (lower_while_ok): New method
This introduces a WIP lowering operation, abstracting away quite a bit of
the manual wiring work, which is really important to providing an API that
provides the proper level of abstraction for actually understanding what the
system is doing.

This does not yet have tests associated with it---I had started, but it's a
lot of work and boilerplate for something that is going to
evolve.  Generally, I wouldn't use that as an excuse, but the robust type
definitions in play, combined with the tiny amount of actual logic, provide
a pretty high level of confidence.  It's very difficult to wire these types
together and produce something incorrect without doing something obviously
bad.

Similarly, I'm holding off on proper docs too, though I did write some
information here.

More to come, after I actually get to work on the XmloReader.

On a side note: I'm happy to have made progress on this, since this wiring
is something I've been dreading and wondering about since before the Parser
abstraction even existed.

Note also that this makes parser::feed_toks private again---I don't intend
to support push parsers yet, since they're only needed internally.  Maybe
for error recovery, but I'll wait to decide until it's actually needed.

DEV-10863
2022-03-23 14:31:16 -04:00
Mike Gerwitz b4a7591357 tamer: obj::xmlo::reader: Begin conversion to ParseState
This begins to transition XmloReader into a ParseState.  Unlike previous
changes where ParseStates were composed into a single ParseState, this is
instead a lowering operation that will take the output of one Parser and
provide it to another.

The mess in ld::poc (...which still needs to be refactored and removed)
shows the concept, which will be abstracted away.  This won't actually get
to the ASG in order to test that that this works with the
wip-xmlo-xir-reader flag on (development hasn't gotten that far yet), but
since it type-checks, it should conceptually work.

Wiring lowering operations together is something that I've been dreading for
months, but my approach of only abstracting after-the-fact has helped to
guide a sane approach for this.  For some definition of "sane".

It's also worth noting that AsgBuilder will too become a ParseState
implemented as another lowering operation, so:

  XIR -> XIRF -> XMLO -> ASG

These steps will all be streaming, with iteration happening only at the
topmost level.  For this reason, it's important that ASG not be responsible
for doing that pull, and further we should propagate Parsed::Incomplete
rather than filtering it out and looping an indeterminate number of times
outside of the toplevel.

One final note: the choice of 64 for the maximum depth is entirely
arbitrary and should be more than generous; it'll be finalized at some point
in the future once I actually evaluate what maximum depth is reasonable
based on how the system is used, with some added growing room.

DEV-10863
2022-03-22 14:06:52 -04:00
Mike Gerwitz f6957ff028 tamer: parse::Parser: Extract logic from Iterator impl
This introduces a (still-private) way to _push_ tokens into the parser,
rather than relying purely on a pull-based interface.  Not only does this
simplify the iterator, but this is also preparing to make the new `feed_tok`
public so that parsers can be composed in more contexts.  I suspect that
this method may also be useful for error recovery, since it can be used to
inject tokens into arbitrary points of a token stream.

I kept the new method private for now so that I can introduce the new API
and docs separate from this refactoring.

DEV-10863
2022-03-22 10:10:59 -04:00
Mike Gerwitz ceb00c4df5 tamer: xir: Complete parse type migration
A previous commit moved the parser.  This updates the types so that they can
actually be utilized in that context.

DEV-10863
2022-03-21 15:50:43 -04:00
Mike Gerwitz 14638a612f tamer: {xir::=>}parse: Move parser out of XIR
The parsing framework originally created for XIR is now more general and
useful to other things.  We'll see how this evolves.

This needs additional documentation, but I'd like to see how it changes as
I implement XmloReader and then some of the source readers first.

DEV-10863
2022-03-18 16:24:53 -04:00
Mike Gerwitz 0360226caa tamer: xir::parse: Generalize input token type
This adds a `Token` type to `ParseState`.  Everything uses `xir::Token`
currently, but `XmloReader` will use `xir::flat::Object`.

Now that this has been generalized beyond XIR, the parser ought to be
hoisted up a level.

DEV-10863
2022-03-18 15:26:05 -04:00
Mike Gerwitz 150b3b9aa4 tamer: xir::flat: Improve parser validation
This does a couple of things: it ensures that documents one and only one
root note, and it properly handles dead transitions once parsing is
complete (allowing it to be composed).

This should make XIRF feature-complete for the time being.  It does rely on
the assumption that the reader is stripping out any trailing whitespace, so
I guess we'll see if that's true as we proceed.

DEV-10863
2022-03-17 23:22:38 -04:00
Mike Gerwitz f04d845452 tamer: xir::flat::parse_token: Remove now-unapplicable comment
Forgot to delete this in a previous commit.

DEV-10863
2022-03-17 21:37:05 -04:00
Mike Gerwitz aba89f809d tamer: xir::parse: UnexpectedEof Span at final offset
I'm not rendering errors yet in practice, so this wouldn't have been
noticed, but we want error messages to reference the final byte in a file on
EOF, not the offset of the last-encountered token, which would be confusing.

This doesn't _directly_ pertain to what I'm working on; I just happened to
notice it.

DEV-10863
2022-03-17 21:33:05 -04:00
Mike Gerwitz e18eb2a4ac tamer: xir::flat::State::parse_node: Use TransitionResult
This was simply missed in a previous commit.

DEV-10863
2022-03-17 16:30:35 -04:00
Mike Gerwitz 6b8f0663ea tamer: xir::{tree::=>}attr: Move
With the introduction of XIRF, attribute parsing is no longer a XIRT thing.

DEV-10863
2022-03-17 16:10:56 -04:00
Mike Gerwitz 7b6d68af85 tamer: xir::parse::Transition: Generalize flat::Transition
XIRF introduced the concept of `Transition` to help document code and
provide mental synchronization points that make it easier to reason about
the system.  I decided to hoist this into XIR's parser itself, and have
`parse_token` accept an owned state and require a new state to be returned,
utilizing `Transition`.

Together with the convenience methods introduced on `Transition` itself,
this produces much clearer code, as is evidenced by tree::Stack (XIRT's
parser).  Passing an owned state is something that I had wanted to do
originally, but I thought it'd lead to more concise code to use a mutable
reference.  Unfortunately, that concision lead to code that was much more
difficult than necessary to understand, and ended up having a net negative
benefit by leading to some more boilerplate for the nested types (granted,
that could have been alleviated in other ways).

This also opens up the possibility to do something that I wasn't able to
before, which was continue to abstract away parser composition by stitching
their state machines together.  I don't know if this'll be done immediately,
but because the actual parsing operations are now able to compose
functionally without mutability getting the way, the previous state coupling
issues with the parent parser go away.

DEV-10863
2022-03-17 16:02:05 -04:00
Mike Gerwitz 899fa79e59 tamer: xir::flat: Initial XIRF implementation
This introduces XIR Flat (XIRF), which is conceptually between XIR and
XIRT.  This provides a more appropriate level of abstraction for further
lowering operations to parse against, and removes the need for other parsers
to perform their own validations (inappropriately) to ensure well-formed
XML.

There is still some cleanup worth doing, including moving some of the
parsing responsibility up a level back into the XIR parser.

DEV-10863
2022-03-17 13:08:16 -04:00
Mike Gerwitz ce48a654b1 tamer: span::Span::offset_add: Make const
This behavior is unchanged, but it allows us to create more constant spans
for testing.  For example:

  const S = DUMMY_SPAN.offset_add(1).unwrap();

This, in turn, will allow for removing lazy_static! for tests that use it
for span generation.

DEV-10863
2022-03-16 14:16:28 -04:00
Mike Gerwitz 18cb5e7b39 tamer: Update dependencies
Petgraph was previously held back due to petgraph-graphml.  I'd like to
transition away from that at some point, given that it's tied to petgraph
and also pulls in xmlns, on top of quick-xml and our XIR, but that can come
down the line.
2022-03-11 10:51:51 -05:00
Mike Gerwitz 2f703ab2df tamer: obj::xmlo: Remove PackageAttrs in favor of token stream
The Options here are awkward and will be able to go away in the new reader
and in AsgBuilder once it has a proper state machine.

This gets rid of some of the initial migratory work for the new reader,
because PackageAttrs is gone.  I'm going to wait to update this to the new
way until I get further into this.

DEV-11449
2022-03-10 15:44:54 -05:00
Mike Gerwitz d428755a2e tamer: obj::xmlo::XmloEvent::SymDeps: Remove
This is not longer needed after the previous commit.
2022-03-10 13:43:07 -05:00
Mike Gerwitz dcfae8a624 tamer: obj::xmlo: Begin transition to streaming quick-xml reader
I'm finally back to TAMER development.

The original plan, some time ago, was to gate an entirely new XmloReader
behind a feature flag (wip-xmlo-xir-reader), and go from there, leaving the
existing implementation untouched.  Unfortunately, it became too difficult
and confusing to marry the old aggregate API with the new streaming one.

AsgBuilder is the only system interacting with XmloReader, so I decided (see
previous commits) to just go the route of refactoring the existing
one.  I'm not yet sure if I'll continue to progressively refactor this one
and eliminate the two separate implementations behind the flag, or if I'll
get this API similar and then keep the flag and reimplement it.  But I'll
know soon.

DEV-11449
2022-03-10 13:31:24 -05:00
Mike Gerwitz 74ddc77adb tamer: xir::escape::CachingEscaper: allow(dead_code) for feature-flagged code
For now, until this feature flag is removed, so that we do not see warnings
when the flag is off.
2022-03-10 10:03:07 -05:00
Mike Gerwitz 76b16fed09 tamer: iter::collect::TryCollect::try_collect_ok: Disambiguate try_collect
The Rust team has begun to introduce try_collect.  I will keep an eye on
this implementation and revisit this, but for the time being, I'm going to
disambiguate this so that I can move on without worrying about a future
breakage.

  - https://github.com/rust-lang/rust/issues/94047
  - https://doc.rust-lang.org/nightly/std/iter/trait.Iterator.html#method.try_collect
2022-03-08 12:55:54 -05:00
Mike Gerwitz 21770305f9 RELEASES.md: Update for v19.0.2 2022-03-07 12:24:31 -05:00
Mike Gerwitz 70d1ad17b8 map: Force param/@default in translation to be numeric
The default ought to be numeric, always, but until we have the compiler
checking for that, I'm going to leave the casting in place.

DEV-10484
2022-03-07 12:22:18 -05:00
Mike Gerwitz 054ad9b4c4 map: Properly apply param/@default for translation fallback
This was broken by the previous fix, because I had cast to a numeric value
before invoking `set_defaults`, which needs the empty string retained so
that it knows whether a default ought to be applied.

This also ensures that `set_values` will always return a numeric value when
that default is applied.

DEV-10484
2022-03-07 11:47:58 -05:00
Mike Gerwitz a49dd68cfd RELEASES.md: Update for v19.0.1 2022-03-03 13:47:37 -05:00
Mike Gerwitz 501a9441a5 map: Produce 0 instead of NaN for non-numeric string values
This has been a problem for...ever, but the old classification system (and
calculations) had `||0` for ever variable reference, whereas the new one
does not; NaNs result in undefined behavior in the new classification
system, since those values are not expected to exist.

This ought to have automated tests, but it will be rewritten in TAMER.

DEV-10484
2022-03-03 13:22:24 -05:00
Mike Gerwitz fb5f38d14c RELEASES.md: Update for v19.0.0 2022-03-01 16:32:43 -05:00
Mike Gerwitz 297b88c3c1 x/0=0 with global flag for new classification system
This was originally my plan with the new classification system, but it was
undone because I had hoped to punt on the somewhat controversial
issue.  Unfortunately, I see no other way.  Here I attempt to summarize the
reasons why, many of which are specific to the design decisions of TAME.

Keep in mind that TAME is a domain-specific language (DSL) for writing
insurance rating systems.  It should act intuitively for our use case, while
still being mathematically sound.

If you still aren't convinced, please see the link at the bottom.

Target Language Semantics (ECMAScript)
--------------------------------------
First: let's establish what happens today.  TAME compiles into ECMAScript,
which uses IEEE 754-2008 floating-point arithmetic.  Here we have:

  x/0 = Infinity,  x > 0;
  x/0 = -Infinity, x < 0;
  0/0 = NaN,       x = 0.

This is immediately problematic: TAME's calculations must produce concrete
real numbers, always.  NaN is not valid in its domain, and Infinity is of no
practical use in our computational model (TAME is build for insurance rating
systems, and one will never have infinite premium).  Put plainly: the
behavior is undefined in TAME when any of these values are yielded by an
expression.

Furthermore, we have _three different possible situations_ depending on
whether the numerator is positive, negative, or zero.  This makes it more
difficult to reason about the behavior of the system, for values we do not
want in the first place.

We then have these issues in ECMAScript:

  Infinity  * 0 = NaN.
  -Infinity * 0 = NaN.
  NaN       * 0 = NaN.

These are of particular concern because of how predicates work in TAME,
which will be discussed further below.  But it is also problematic because
of how it propagates: once you have NaN, you'll always have NaN, unless you
break out of the situation with some control structure that avoids using it
in an expression at all.

Let's now consider predicates:

  NaN  >  0   = false.
  NaN  <  0   = false.
  NaN === 0   = false.
  NaN === NaN = false.

These will be discussed in terms of classification predicates (matches).

We also have issues of serialization:

  JSON.stringify(Infinity) = "null".
  JSON.stringify(NaN)      = "null".

These means that these values are difficult to transfer between systems,
even if we wanted them.

TAME's Predicates
-----------------
TAME has a classification system based on first-order logic, where ⊥ is
represented by 0 and ⊤ is represented by 1.  These classifications are used
as predicates to calculations via the @class attribute of a rate block.  For
example:

  <rate-each class="property" generates="propValue" index="k">
    <c:quotient>
      <c:value-of name="buildingTiv" index="k" />
      <c:value-of name="tivPropDivisor" index="k" />
    </c:quotient>
  </rate>

As can be observed via the Summary Page, this calculation compiles into the
following mathematical expression:

  ∑ₖ(pₖ(tₖ/dₖ)),

that is—the quotient is then multiplied by the value of the `property`
classification, which is a 0 or 1 respectively for that index.

Let's say that tivPropDivisor were defined in this way:

  <rate-each class="property" generates="tivPropDivisor" index="k">
    <!--- ... logic here ...  -->
  </rate>

It does not matter what the logic here is.  Observe that the predicate here
is `property` as well, which means that, if this risk is not a property
risk, then `tivPropDivisor` will be `0`.

Looking back at `propValue`, let's say that we do have a property risk, and
that `buildingTiv` is `[100_000, 200_000]` and `tivPropDivisor` is 1000.  We
then have:

  1(100,000 / 1000) + 1(200,000 / 1000)) = 300.

Consider instead what happens if `property` is 0.  Since we have no property
locations, we have `[0, 0]` as `buildingTiv` and `tivPropDivisor` is 0.

  0(0/0) + 0(0/0)) = 0(NaN + NaN) = NaN.

This is clearly not what was intended.  The predicate is expected to be
_strongly_ zero, as if using an Iverson bracket:

  ((0/0)[0] + (0/0)[0]) = 0.

Of course, one option is to redefine TAME such that we use Iverson's
convention in place of summation, however this is neither necessary nor
desirable given that

  (a) NaN is not valid within the domain of any TAME expression, and
  (b) Summation is elegantly generalized and efficiently computed using
      vector arithmetic and SIMD functions.

That is: there's no use in messing with TAME's computational model for a
valid that should be impossible to represent.

Short-Circuiting Computation
----------------------------
There's another way to look at it, though: that we intended to skip the
computation entirely, and so it doesn't matter what the quotient is.  If the
compiler were smart enough (and maybe one day it will be), it would know
that the predicate of `tivPropDivisor` and `propValue` are the same and so
there is no circumstance under which we would compute `propValue` and have
`tivPropDivisor` be 0.

The problem is: that short-circuiting is employed as an _optimization_, and
is an implementation detail.  Mathematically, the expression is unchanged,
and is still invalid within TAME's domain.  It is unrepresentable, and so
this is not an out.

But let's pretend that it was defined that way, which would yield this:

              { ∑ₖ(pₖ(tₖ/dₖ)),  ∀x∈p(x = 1);
  propValue = <
              { 0,             otherwise.

This is the optimization that is employed, but it's still not mathematically
correct!  What happens if p₀ = 1, but p₁ = 0?  Then we have:

  1(100,000/1000) + 0(0/0) = 100 + NaN = NaN,

but the _intent_ was clearly to have 100 + 0 = 100, and so we return to the
original problem once again.

Classification Predicates and Intent
------------------------------------
Classifications are used as predicates for equations, but classifications
_themselves_ have predicates in the form of _matches_.  Consider, for
example, a classification that may be used in an assertion to prevent
negative premium from being generated:

  <t:assert failure="premBuilding must not be negative for any index">
    <t:match-gte value="premBuilding" value="#0" />
  </t:assert>

Simple enough—the system will fail if the premium for a given building is
below $0.

But what happens if premBuilding is calculated as so?

  <rate-each class="property" yields="premBuildingTotal"
             generates="premBuilding" index="k">
    <c:product>
      <c:value-of name="propValue" index="k" />
      <c:value-of name="propRate" index="k" />
    </c:product>
  </rate-each>

Alas, if `property` is false for any index, then we know that `propValue` is
NaN, and NaN * x = NaN, and so `premBuilding` is NaN.

The above assertion will compile the match into the first-order sentence

  ∀x∈b(x > 0).

Unfortunately, NaN is not greater than, less than, equal to, or any other
sort of thing to 0, and so _this assertion will trigger_.  This causes
practical problems with the `_premium_` template, which has an
`@allow-zero@` argument to permit zero premium.

Consider this real-world case that I found (variables renamed), to avoid a
strawman:

  <t:premium class="loc" round="cent"
             yields="locInitialTotal"
             generates="locInitial" index="k"
             allow-zero="true"
             desc="...">
    <c:value-of name="premAdditional" />

    <c:quotient>
      <c:value-of name="premLoc" index="k" />
      <c:value-of name="premTotal" />
    </c:quotient>
  </t:premium>

This appears to be responsible for splitting up `premAdditional` relative to
the total premium contribution of each location.  It explicitly states that
it wants to permit a zero value.  The intent of this block is clear: a value
of 0 is explicitly permitted and _expected_.

But if `premTotal` is for whatever reason 0—whether it be due to a test
case or some unexpected input—then it'll yield a NaN and make the entire
expression NaN.  Or if `premAdditional` or `premLoc` are tainted by a NaN,
the same result will occur.  The assertion will trigger.  And, indeed, this
is what I'm seeing with test cases against the new classification system.

What about Infinity?  Is it intuitive that, should `propValue` in the
previous example be positive and `propRate` be 0, that we would, rather than
producing a very small value, produce an infinitely large one?  Does that
match intuition?  Remember, this system is a domain-specific language for
_our_ purposes—it is not intended to be used to model infinities.

For example, say we had this submission because the premium exceeds our
authority to write with some carrier:

  <t:submit reason="Premium exceeds authority">
    <t:match-gt name="premBuilding" value="#100k" />
  </t:submit>

If we had

  (100,000 / 0) = ∞,

then this submit reason would trigger.  Surely that was not intended, since
we have `property` as a predicate and `propRate` with the same predicate,
implying that the answer we _actually_ want is 0!  In that case, what we
_probably_ want to trigger is something like

  <rate yields="premFinal">
    <t:maxreduce>
      <c:value-of name="premBuildingTotal" />
      <c:value-of name="#500" />
    </t:maxreduce>
  </rate>,

in order to apply a minimum premium of $500.  But if `premBuildingTotal` is
Infinity, then you won't get that—you'll get Infinity, which is of course
nonsense.

And nevermind -Infinity.

Why Wasn't This a Problem Before?
---------------------------------
So why bring this up now?  Why have we survived a decade without this?

We haven't, really—these bugs have been hidden.  But the old classification
system covered them up; predicates would implicitly treat missing values as
0 by enclosing them in `(x||0)` in the compiled code.  Observe this
ECMAScript code:

  NaN || 0 = 0.

Consequently, the old classification system absorbed bad values and treated
them implicitly as 0.  But that was a bug, and had to be removed; it meant
that missing indexes in classifications would trigger predicates that were
not intended to be triggered, if they matched against 0, or matched against
a value less than some number larger than zero.  (See
`core/test/core/class` for examples.)

The new classification system does not perform such defaulting.  _But it
also does not expect to receive values outside of its valid domain._
Consequently, _NaN and Infinity lead to undefined behavior_, and the
current implementation causes the predicate to match (NaN < 0) and therefore
fail.

The reason for this is because that this implementation is intended to
convey precisely the computation necessary for the classification system, as
formally defined, so that it can be later optimized even further.  Checking
for values outside the domain not only should not be necessary, but it would
prevent such future optimizations.

Furthermore, parameters used to compile into (param||0), to account for
missing values or empty strings.  This changed somewhat recently with
5a816a4701, which pre-cast all inputs and
allowed relaxing many of those casts since they were both wasteful and no
longer necessary.

Given that, for all practical purposes, 0/0=0 in the system <1yr ago.

Infinity, of course, is a different story, since (Infinity||0)=Infinity;
this one has always been a problem.

Let's Just Fail
---------------
Okay, so we cannot have a valid expression, so let's just fail.

We could mean that in two different ways:

  1. Fail at runtime if we divide by 0; or
  2. Fail at compile-time if we _could_ divide by 0.

Both of these have their own challenges.

Let's dismiss #2 right off the bat for now, because until we have TAMER,
that's not really feasible.  We need something today.  We will discuss that
in the future.

For #1—we cannot just throw an error and halt computation, because if the
`canterm` flag passed into the system is `false`, then _computation must
proceed and return all results_.  Terminating classifications are checked
after returning rather than throwing errors.

Since we have to proceed with computation, then the computations have to be
valid, and so we're left with the same problem again—we cannot have
undefined behavior.

One could argue that, okay, we have undefined behavior, but we're going to
fail because of the assertion anyway!  That's potentially defensible, but it
is at the moment undesirable, because we get so many failures.  And,
relative to the section below, it's not clear to me what benefit we get from
that behavior other than making things more difficult for ourselves.

Furthermore, such an assertion would have to be defined for every
calculation that performs a quotient, and would have to set some
intermediate flag in the calculation which would then have to be checked for
after-the-fact.  This muddies the generated calculation, which causes
problems for optimizations, because it requires peering into state of the
calculation that may be hidden or optimized away.

If we decide that calculations must be valid because we cannot fail, and we
have to stick with the domain of calculations, then `x/0` must be
_something_ within that domain.

x/0=0 Makes Sense With the Current System
-----------------------------------------
Let's take a step back.  Consider a developer who is unaware that
NaN/Infinity are permitted in the system—they just know that division by
zero is a bad thing to do because that's what they learned, and they want to
avoid it in their code.

Consider that they started with this:

  <rate-each class="property" generates="propValue" index="k">
    <c:quotient>
      <c:value-of name="buildingTiv" index="k" />
      <c:value-of name="tivPropDivisor" index="k" />
    </c:quotient>
  </rate>

They have inspected the output of `tivPropDivisor` and see that it is
sometimes 0.  They understand that `property` is a predicate for the
calculation, and so reasonably think that they could do something like this:

  <classify as="nonzero-tiv-prop-divisor" ...>
    <t:match-ne on="tivPropDivisor" value="#0" />
  </classify>

and then change the rate-each to

  <rate-each class="property nonzero-tiv-prop-divisor" ...>.

Except that, of course, we know that will have no effect, because a NaN is a
NaN.  This is not intuitive.

So they'd have to do this:

  <rate-each class="property" generates="propValue" index="k">
    <c:cases>
      <c:case>
        <t:when-ne name="tivPropDivisor" value="#0" />

        <c:quotient>
          <c:value-of name="buildingTiv" index="k" />
          <c:value-of name="tivPropDivisor" index="k" />
        </c:quotient>
      </c:case>

      <c:otherwise>
        <c:value-of name="#0" />
      </c:otherwise>
    </c:cases>
  </rate>.

But for what purpose?  What have we gained over simply having x/0=0, which
does this for you?

The reason why this is so unintuitive is because 0 is the default case in
every other part of the system.  If something doesn't match a predicate, the
value becomes 0.  If a value at an index is not defined, it is implicitly
zero.  A non-matching predicate is 0.

This is exploited for reducing values using summation.  So the behavior of
the system with regards to 0 is always on the mind of the developer.  If we
add it in another spot, they would think nothing of it.

It would be nice if it acted as an identity in a monoidic operation,
e.g. as 0 for sums but as 1 for products, but that's not how the system
works at all today.  And indeed such a thing could be introduced using a
special template in place of `c:value-of` that copies the predicates of the
referenced value and does the right thing.

The _danger_, of course, is that this is _not_ how the system as worked, and
so changing the behavior has the risk of breaking something that has relied
on undefined behavior for so long.  This is indeed a risk, but I have taken
some confident in (a) all the test cases for our system pass despite a
significant number of x/0=0 being triggered due to limited inputs, and (b)
these situations are _not correct today_, resulting in `null` in serialized
result data because `JSON.stringify([NaN, Infinity]) === "[null, null]"`.

Given all of that, predictable incorrect behavior is better than undefined
behavior.

So x/0=0 Isn't Bad?
-------------------
No, and it's mathematically sound.  This decision isn't unprecedented—
Coq, Lean, Agda, and other theorem provers define x/0=0.  APL originally
defined x/0=1, but later switched to 0.  Other languages do their own thing
depending on what is right for their particular situation.

Division is normally derived from

  a × a⁻¹ = 1, a ≠ 0.

We're simply not using that definition—when we say "quotient", or use the
`/` symbol, we mean a _different_ function (`div`, in the compiled JS),
where we have an _additional_ axiom that

  a / 0 = 0.

And, similarly,

  0⁻¹ = 0.

So we've taken a _normally undefined_ case and given it a definition.  No
inconsistency arises.

In fact, this makes _sense_ to do, because _this is what we want_.  The
alternative, as mentioned above, is a lot of boilerplate—checking for 0 any
time we want to do division.  Complicating the compiler to check for those
cases.  And so on.  It's easier to simple state that, in TAME, quotients
have this extra convenient feature whereby you don't have to worry about
your denominator being zero because it'll act as though you enclosed it in a
case statement, and because of that, all your code continues to operate in
an intuitive way.

I really recommend reading this blog post regarding the Lean theorem prover:

  https://xenaproject.wordpress.com/2020/07/05/division-by-zero-in-type-theory-a-faq/
2022-02-28 16:27:51 -05:00
Mike Gerwitz 9fa79ce5ea TAME_PARAMS: New Makefile var
This is intended to be set via the configure script, and is being added
primarily for the upcoming flag to enable the legacy classification
system.  This is only used for the XSLT-based compiler.
2022-02-28 12:35:17 -05:00
Mike Gerwitz ce0da76ccf Improve symbol table processing time
preproc:symtable-process-symbols is run on each pass (e.g. during initial
processing and after each template expansion) to introduce new symbols into
the symbol table from imports and newly discovered symbols.

This processing was previously optimized a bit using maps to reduce the cost
of symbol table lookups, but the processing was still inefficient, relying
on XSLT1-style processing (as originally written) for deduplication.  This
now uses `for-each-group` and `perform-sort` to offload the expensive
computation onto Saxon, which is much more efficient.

Symbol table processing has long been a culprit, but I hadn't attempted to
optimize further in recent months because of TAMER work.  Since TAMER has
been on pause for a few months with other things needing my attention, I
needed to provide a short-term performance improvement to keep up with
increasing build times.

DEV-11716
2022-02-22 22:05:07 -05:00
Mike Gerwitz 1796753940 core/vector: Remove aggregate package
Like core/numeric, this was to maintain BC and has not been used for many
years (it does not even build).
2022-01-28 12:01:18 -05:00
Mike Gerwitz a300842582 core/build.xml: Remove
This is no longer necessary (and proably never was).  I assume that this was
added when I was trying to get core to build independently.
2022-01-28 12:00:26 -05:00
Mike Gerwitz 40e2472fac core/numeric: Remove aggregate package
This package was originally added long ago when it was split into
multiple.  It is no longer used.
2022-01-28 11:56:06 -05:00
Mike Gerwitz cd13b80f31 build-aux/check-coupling: Prohibit supplier imports of UI packages
The reverse was checked, but apparently a check for suppliers importing the
UI was never added.
2022-01-28 10:50:27 -05:00
Mike Gerwitz 2a84e44a58 bin/tame: Fix runner output line clearing
The output was being omitted under certain conditions, meaning that users
would have to look in the runlogs for errors.
2022-01-28 09:21:34 -05:00
Mike Gerwitz 8b255c2251 tame: tamed --help: Add missing closing quote to awk example 2022-01-26 13:51:34 -05:00
Mike Gerwitz 8fbddfb3b3 tamed: Fix --help and add another reporting example
$2 was not escaped and would fail expansion.  I apparently did not run
--help before committing.  Shame on me.
2022-01-20 23:32:28 -05:00
Mike Gerwitz 6fd570477a tamed: Add runtab and TAMED_RUNTAB_OUT
This provides logging that can be used to analyze jobs.  See `tamed --help`
for some examples.  More to come.

You'll notice that one of the examples reprents package build time in
_minutes_.  This is why TAMER is necessary; as of the time of writing, the
longest-building package is nearly five and a half minutes, and there are a
number of packages that take a minute or more.  But, there are potentially
other optimizations that can be done.  And this is _after_ many rounds of
optimizations over the years.  (TAME was not originally built for what it is
currently being used for.)
2022-01-19 16:47:12 -05:00
Mike Gerwitz 4a3b86f480 tamed: Ignore SIGUSR2
This was originally going to tell tamed to redraw the runner status line,
but a different approach was taken.
2022-01-19 15:41:28 -05:00
Mike Gerwitz c72d908a3f tamed: Add missing --report to help
Missing from previous commit.
2022-01-19 13:29:23 -05:00
Mike Gerwitz 756dcd7894 tamed --report and runner status line (TAMED_TUI)
This is something that I've wanted to do for quite some time, but for good
reason, have been avoiding.

`tamed --report` is fairly basic right now, but allows you to see what each
of the runners are doing.  This will be expanded further to gather data for
further analysis.

The thing that I was avoiding was a status line during the build to
summarize what the runners are doing, since it's nearly impossible to do so
from the build output with multiple runners.  This will not only allow me to
debug more easily, but will keep the output plainly visible to developers at
all times in the hope that it can help them improve the build times
themselves in certain cases.

It is currently gated behind TAMED_TUI, since, while it works well overall,
it is imperfect, and will cause artifacts from build output partly
overwriting the status line, and may even occasionally clobber the PS1 by
erasing the line.  This will be improved upon in the future; something is
better than nothing.
2022-01-19 11:51:48 -05:00