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

New Generate Modes #250

Merged
merged 4 commits into from
Jan 18, 2025
Merged

New Generate Modes #250

merged 4 commits into from
Jan 18, 2025

Conversation

sepandhaghighi
Copy link
Owner

Reference Issues/PRs

What does this implement/fix? Explain your changes.

  • 2 new generation modes added
    1. F1F2_VS_F1
    2. F1F2_VS_F2

Any other comments?

@sepandhaghighi sepandhaghighi self-assigned this Jan 16, 2025
@sepandhaghighi sepandhaghighi added the enhancement New feature or request label Jan 16, 2025
@sepandhaghighi sepandhaghighi added this to the samila v1.5 milestone Jan 16, 2025
Copy link

codecov bot commented Jan 16, 2025

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.46%. Comparing base (5dbf2d3) to head (cfa38ca).
Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
samila/genimage.py 83.34% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #250      +/-   ##
==========================================
+ Coverage   99.46%   99.46%   +0.01%     
==========================================
  Files           4        4              
  Lines         730      738       +8     
  Branches      109      111       +2     
==========================================
+ Hits          726      734       +8     
  Misses          2        2              
  Partials        2        2              

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

@sepandhaghighi sepandhaghighi marked this pull request as ready for review January 16, 2025 16:29
Comment on lines +148 to +153
elif self.generate_mode == GenerateMode.F1F2_VS_F1.value:
data1_ = self.function1(point[0], point[1]).real * self.function2(point[0], point[1]).real
data2_ = self.function1(point[0], point[1]).real
elif self.generate_mode == GenerateMode.F1F2_VS_F2.value:
data1_ = self.function1(point[0], point[1]).real * self.function2(point[0], point[1]).real
data2_ = self.function2(point[0], point[1]).real
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if these are food atomic modes. If we add this we should add F1_VS_F1F2, F1_VS_F2F1, and F1+F2_VS_F1 etc. Therefore I think that's not a good idea to add this mode.

Copy link
Owner Author

Choose a reason for hiding this comment

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

If it doesn't produce a beautiful mode, it is also not harmful.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know. I wanted this set to be atomic, or at least closed under the operations we're defining (including all cases).

Copy link
Owner Author

Choose a reason for hiding this comment

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

I understand your point, and if you're okay with it, I will add F1_VS_F1F2 and F1_VS_F2F1 in the next version of Samila. I generated this image using the F1F2_VS_F1 mode:

test

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, that's beautiful. I guess that's fine then.

@sadrasabouri sadrasabouri merged commit a11c96a into dev Jan 18, 2025
19 of 20 checks passed
@sadrasabouri sadrasabouri deleted the new_generate_modes branch January 18, 2025 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants