Fix to issue #124 #125
Merged
Fix to issue #124 #125
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
|
great, thanks. BTW, if you are able to specify a |
|
sorry for the delayed response, busy day. If you would like, I can still add the test_params in another PR. |
|
sure!
…On Mon, May 16, 2022, 6:09 PM nightdr ***@***.***> wrote:
sorry for the delayed response, busy day. If you would like, I can still
add the test_params in another PR.
—
Reply to this email directly, view it on GitHub
<#125 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABSGCGWWEFOBJ6EUMTIJAH3VKLBRFANCNFSM5WCTUFOA>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
|
sounds good, just added a new pull request and the CI times should be significantly shorter now. |
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.
I fixed the timeout issue in our source code and added more slack to cases where n<=1000 since ideally the timeout shouldn't be happening.
Also the provided string replacement code has a potential issue: say we have the model
X_10 + X_1with the mappingX_0: 'a', X_1: 'b', ... 'X_10': 'k', the provided code will turnX_10 + X_1intob0 + brather than the intendedk + bsince it will replace allX_1instances withb. To fix this, I iterated through the variables in reversed order (see the commit). I remember seeing this code used in at least one other submission, so this might be an issue if methods are responsible for doing their own variable name replacement.