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.

Sign up for GitHub

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

Jump to bottom

[tmva][sofie] Add Sin and Cos Operators to SOFIE #16809

Closed
wants to merge 1 commit into from
Closed

[tmva][sofie] Add Sin and Cos Operators to SOFIE #16809

wants to merge 1 commit into from

Conversation

extint
Copy link
Contributor

@extint extint commented Nov 3, 2024

This Pull request:

This Pull request adds the support for Sin and Cos operators in SOFIE

Changes or fixes:

  • Implemented the respective ROperator files
  • Included respective tests and ONNX models

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 4, 2024

Test Results

    17 files      17 suites   3d 8h 54m 42s ⏱️
 2 663 tests  2 663 ✅ 0 💤 0 ❌
43 575 runs  43 575 ✅ 0 💤 0 ❌

Results for commit 824298e.

lmoneta
lmoneta reviewed Dec 12, 2024
View reviewed changes
Copy link
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

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

Thank you Vedant for the PR.
I think for simplicity, it is better to implement Sin and Cos operator as additional Unary operators, as we are currently doing for Log, Exp and Sqrt.

I will make a new PR , but re-using your test code

lmoneta added a commit to lmoneta/root that referenced this pull request Dec 12, 2024
@lmoneta
[tmva][sofie] Add Sin/Cos operators
812964f 
Add Sin and Cos operators as new Unary operators.
Add also tests, taken from Vedant's PR  root-project#16809
@lmoneta lmoneta mentioned this pull request Dec 12, 2024
Open
@lmoneta
Copy link
Member

lmoneta commented Dec 12, 2024

This PR is implemented in a commit of #17248

@lmoneta lmoneta closed this Dec 12, 2024
lmoneta added a commit to lmoneta/root that referenced this pull request Dec 13, 2024
@lmoneta
[tmva][sofie] Add Sin/Cos operators
4b12e44 
Add Sin and Cos operators as new Unary operators.
Add also tests, taken from Vedant's PR  root-project#16809
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmoneta lmoneta lmoneta left review comments

@bellenot bellenot Awaiting requested review from bellenot bellenot will be requested when the pull request is marked ready for review bellenot is a code owner

Assignees

@lmoneta lmoneta

Labels
in:TMVA
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@extint @lmoneta @guitargeek