Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to MC and MP #225

Closed
wants to merge 0 commits into from
Closed

Conversation

bryates
Copy link
Contributor

@bryates bryates commented Feb 15, 2022

This PR fixes the MP timing issue, as well as the recent disagreement in the MC.

MC:
All barrel layers run and fully match

MP:
All barrel layers run and fully match
CP achieved post-implementation: 3.837

Test vectors are temporary until fw_sync_12_0_0_pre4 is updated.

@bryates bryates requested a review from aehart February 15, 2022 15:32
@bryates bryates force-pushed the MP_cleanup_220208 branch 2 times, most recently from 7430af0 to 3d512e3 Compare February 15, 2022 15:34
@bryates bryates changed the base branch from master to fw_sync_12_0_0_pre4 February 15, 2022 15:35
@bryates bryates force-pushed the MP_cleanup_220208 branch 2 times, most recently from 88398b6 to 7d33622 Compare February 15, 2022 15:40
@bryates bryates requested a review from aryd February 15, 2022 16:46
@bryates
Copy link
Contributor Author

bryates commented Feb 17, 2022

I've included all barrel layers in download.sh, and tested the for both the MP and MC. All give full agreement. I left the tcl scripts set to only run the PHIC3 regions, so the CI time won't change.

@aehart aehart force-pushed the fw_sync_12_0_0_pre4 branch from f6f8b64 to 8139170 Compare February 18, 2022 01:19
@meisonlikesicecream
Copy link
Contributor

Could you try and rebase this branch w.r.t. fw_sync_12_0_0_pre4? The list of files changed is a bit confusing as it contains changes that were made in the fw synch branch thus shouldn't show up for this PR.

@@ -763,6 +756,11 @@ void MatchCalculator(BXType bx,
id_next = projid;
bool newtracklet = (istep==0 || (id_next != id))? true : false;

//increament full match memories
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

increment? (sorry :-) )

@bryates bryates closed this Feb 18, 2022
@bryates
Copy link
Contributor Author

bryates commented Feb 18, 2022

Could you try and rebase this branch w.r.t. fw_sync_12_0_0_pre4? The list of files changed is a bit confusing as it contains changes that were made in the fw synch branch thus shouldn't show up for this PR.

After the rebase, it closed the PR. I've never seen GitHub do that before. @aehart was this branch already merged into the fw sync?

@bryates bryates reopened this Feb 18, 2022
@aehart aehart force-pushed the fw_sync_12_0_0_pre4 branch from c327f41 to f1a2dc6 Compare February 21, 2022 02:05
@bryates bryates closed this Feb 21, 2022
@aehart
Copy link
Contributor

aehart commented Feb 21, 2022

@bryates I did already add the commits from this branch to the FW sync branch. Sorry, I didn't realize you were using that branch for this PR.

I think these changes are needed on the FW sync branch in order for the MC and MP to pass the C-simulation, so I'll just review them on #220. Other PRs that are not needed for C-simulation to pass can wait until #220 is merged.

@bryates
Copy link
Contributor Author

bryates commented Feb 21, 2022

@bryates I did already add the commits from this branch to the FW sync branch. Sorry, I didn't realize you were using that branch for this PR.

I think these changes are needed on the FW sync branch in order for the MC and MP to pass the C-simulation, so I'll just review them on #220. Other PRs that are not needed for C-simulation to pass can wait until #220 is merged.

Ok, thanks!

@pwittich pwittich deleted the MP_cleanup_220208 branch January 31, 2023 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants