Adding support for multi-alts in graph #783
Merged
Conversation
This file contains 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
JPepperwood123
requested review from
pavpanchekha,
ztatlock,
elmisback and
bksaiki
…upport-code Merge with main
pavpanchekha
approved these changes
Apr 29, 2024
| ; error: object with fields {start, target, end}, where each field holds | ||
| ; an array like [y0, ..., ym] where y0 etc are bits of error for the output | ||
| ; on each point | ||
| ; error: JSON dictionary where keys are {start, end, target1, ..., targetn}. |
There was a problem hiding this comment.
I mean, this is ok, but isn’t it kind of ugly? Why not make one entry, target, point to an array?
There was a problem hiding this comment.
My idea as to generalize the report.js as much as possible. This made it one for-loop instead of multiple. And multiple start and end seemed of less probability. I could revert it, but it was to make the JS much less easier to read
added 2 commits
May 9, 2024 15:41
…upport-code Merge with main
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.
The previous multi-alts PR incorporated the support for multi-alts in all the .rkt files as the equation finally gets displayed.
However, despite all the error values being generated, only one singular target (the best one) was being selected across all the Developer Targets.
This PR attempts to include all Developer Targets in the graph where the values are not only displayed, but are multi-colored to an extent to look unique.
This involved changing pages.rkt and report.js to allow not one target, but an array of developer targets, which required removing a lot of hardcoding.