Repository navigation
refactor: template SScalar on the derivative order - #87
Merged
Merged
Conversation
Mirrors DDScalar, where the order is the first template parameter and the Python name carries one letter per order -- DScalar and DDScalar. SScalar becomes SScalar<TOrder, TScalar>, constrained to order 1 for now since second order is not implemented yet, so SScalar<2, double> is a clear error rather than a silent instantiation. Mechanical: 25 template heads and 55 type uses in the header, one site in the bindings, seven in the tests. No behaviour change, which the characterization suite confirms without a single expected value being touched. This breaks hyperjet::SScalar<double> for C++ callers, who now write hyperjet::SScalar<1, double>. Putting the order first is what makes the two class templates consistent; the alternative, ordering it <TScalar, TOrder> to keep the old spelling working, would have left the library with two different conventions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Groundwork for named second-order derivatives. On its own this changes no behaviour — it makes room for one.
What
SScalar<TScalar>becomesSScalar<TOrder, TScalar>, mirroringDDScalar, where the order is the first template parameter and the Python name carries one letter per order —DScalarandDDScalar.The constraint is
requires(TOrder == 1): second order is not implemented in this PR, soSScalar<2, double>is a clear constraint violation rather than a silent instantiation that would compute a zero Hessian.Mechanical throughout: 25 template heads and 55 type uses in the header, one site in the bindings, seven in the tests. No behaviour change, which the characterization suite from #84 confirms without a single expected value being touched.
The C++ spelling changes
hyperjet::SScalar<double>becomeshyperjet::SScalar<1, double>. Putting the order first is what makes the two class templates consistent. The alternative — ordering it<TScalar, TOrder>so the old spelling keeps working — would have left the library with two different conventions for the same concept. The Python namehj.SScalaris unchanged.Verification
-Werrorhj.SScalarstill exposes the same 35 methodsclang-format --WerrorcleanWhere this is going
Second order is a dense triangular Hessian over the sorted names from #85 — the position of a name in that vector is the index the Hessian is laid out over. The design is settled and the groundwork done:
merge_namesproduces the union in one pass and records where each operand's names land, which is what the Hessian has to be scattered alongDDScalaruses, where 1473 assertions already cover itDDScalarfunctions rather than transcribed by handNot in this PR, and deliberately so. A first attempt at wiring the 29 call sites used a script that collected all regex matches up front and then replaced them sequentially; after the first replacement the later search strings no longer existed, and
str.replacebecame a silent no-op. It reportedatan2as converted when it was not. Left unnoticed, that would have shipped functions returning a silently zero Hessian — the worst failure mode for an AD library, since it computes rather than crashes.So the follow-up writes symbolically generated second-order expectations for all 24 functions before the conversion, so a missed one fails a test instead of nothing at all.