Fix --debug-generate-data omitting newer constraints#1588
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1588 +/- ##
==========================================
- Coverage 90.51% 90.34% -0.17%
==========================================
Files 65 65
Lines 10000 10027 +27
==========================================
+ Hits 9051 9059 +8
- Misses 949 968 +19
🚀 New features to boost your workflow:
|
| | Transformation.Upper ({meta= {type_= UVector | URowVector; _}; _} as e) -> | ||
| gen_bounded m (fun x -> gen_num_real m (Transformation.Upper x)) e | ||
| | Upper ({meta= {type_= UVector | URowVector | UReal | UInt; _}; _} as e) -> | ||
| gen_bounded m n (fun x -> gen_num_real m (Upper x)) e |
There was a problem hiding this comment.
Since gen_bounded can now handle any bounds that pass typecheck, there's no reason to match on type_ at all.
| | Identity | Offset _ | Multiplier _ | OffsetMultiplier _ | CholeskyCorr | ||
| |CholeskyCov | Correlation | Covariance | StochasticRow | StochasticColumn | ||
| |TupleTransformation _ | Lower _ | LowerUpper _ | Upper _ -> | ||
| Expr.Helpers.transpose (gen_row_vector m n t) |
There was a problem hiding this comment.
Some of these are just nonsense; we don't have Cholesky or covariance vectors. These should ICE.
There was a problem hiding this comment.
They do ice inside gen_row_vector, I was debating if it was worth duplicating.
There was a problem hiding this comment.
The purpose of listing all transformations individually is to be explicit about which ones are valid, I think.
| let exprs = | ||
| match Expr.Helpers.try_unpack evaled with | ||
| | Some unpacked -> unpacked | ||
| | None -> List.init n ~f:(Fn.const evaled) in |
There was a problem hiding this comment.
List.init is basically the same as our repeat_th, right? And this could be just repeat.
There was a problem hiding this comment.
Our repeat_th expects a unit -> 'a, not a int -> 'a, but it's also more efficiently implemented so I've re-defined repeat_th in terms of List.init and then made sure we're consistently using it
2 participants
This also exhaustively lists the variants of Transformation.t, so it will be harder to forget in the future...
Submission Checklist
Release notes
--debug-generate-datais now aware of the newsum_to_zero_vector,sum_to_zero_matrix,stochastic_row_matrix, andstochastic_column_matrixtypes.Copyright and Licensing
By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)