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

Multithreading Support #231

Closed

Conversation

olynch
Copy link
Collaborator

@olynch olynch commented Aug 16, 2024

@olynch
Copy link
Collaborator Author

olynch commented Aug 16, 2024

This should close #224.

src/Rules.jl Show resolved Hide resolved
src/EGraphs/saturation.jl Outdated Show resolved Hide resolved
@0x0f0f0f
Copy link
Member

@olynch can you please push another commit to see if now CI runs also on these PRs?

@nmheim
Copy link

nmheim commented Aug 22, 2024

Hey hey I just looked into why the benchmark workflows are not working. Quote from here):

In public repositories this action does not work in pull_request workflows when triggered by forks. Any attempt will be met with the error, Resource not accessible by integration. This is due to token restrictions put in place by GitHub Actions. Private repositories can be configured to enable workflows from forks to run without restriction. See here for further explanation. Alternatively, use the pull_request_target event to comment on pull requests.

We could do the following (quote from here):

Use a repo scoped Personal Access Token (PAT) created on an account that has write access to the repository that pull requests are being created in. This is the standard workaround and recommended by GitHub. However, the PAT cannot be scoped to a specific repository so the token becomes a very sensitive secret. If this is a concern, the PAT can instead be created for a dedicated machine account that has collaborator access to the repository. Also note that because the account that owns the PAT will be the creator of pull requests, that user account will be unable to perform actions such as request changes or approve the pull request.

(I am not saying that we should do this here, just wanted to leave it in a comment so it does not get lost:)

@0x0f0f0f
Copy link
Member

Hey hey I just looked into why the benchmark workflows are not working. Quote from here):

In public repositories this action does not work in pull_request workflows when triggered by forks. Any attempt will be met with the error, Resource not accessible by integration. This is due to token restrictions put in place by GitHub Actions. Private repositories can be configured to enable workflows from forks to run without restriction. See here for further explanation. Alternatively, use the pull_request_target event to comment on pull requests.

We could do the following (quote from here):

Use a repo scoped Personal Access Token (PAT) created on an account that has write access to the repository that pull requests are being created in. This is the standard workaround and recommended by GitHub. However, the PAT cannot be scoped to a specific repository so the token becomes a very sensitive secret. If this is a concern, the PAT can instead be created for a dedicated machine account that has collaborator access to the repository. Also note that because the account that owns the PAT will be the creator of pull requests, that user account will be unable to perform actions such as request changes or approve the pull request.

(I am not saying that we should do this here, just wanted to leave it in a comment so it does not get lost:)

@olynch due to this, benchmarks are not going to run. I'm going to give you maintainer access so you can have branches here. Would it be possible for you to post the results of benchmarks here and against 3.0 on your machine please?

@codecov-commenter
Copy link

codecov-commenter commented Aug 29, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.09%. Comparing base (8ad3c31) to head (f4fc630).
Report is 33 commits behind head on ale/3.0.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           ale/3.0     #231      +/-   ##
===========================================
+ Coverage    79.62%   82.09%   +2.46%     
===========================================
  Files           19       19              
  Lines         1502     1597      +95     
===========================================
+ Hits          1196     1311     +115     
+ Misses         306      286      -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@olynch olynch closed this Sep 11, 2024
@olynch olynch mentioned this pull request Sep 11, 2024
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.

4 participants