Skip to content

Conversation

@UF4007
Copy link
Contributor

@UF4007 UF4007 commented May 9, 2025

Update perf.cc for mod operation benchmark.
Somehow, the Files changed appear a lot of files you have been merged to branch master.
————————
OK, I rebased the fmod branch to the master branch and reverted some changes in the fmod branch which is after your last merge in the fmod branch. Now it seems to be back on the right track.
I know it may not be the best way, but I have poor experience in such an open source project, I am very sorry. I usually do teamwork on a private git server which have no PR mechanism.

@UF4007 UF4007 added the enhancement New feature or request label May 9, 2025
Copy link
Owner

@arturbac arturbac left a comment

Choose a reason for hiding this comment

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

thre is already fmod function object and function also

}
};

static fixed_t fmod(fixed_t& a, fixed_t& b)
Copy link
Owner

Choose a reason for hiding this comment

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

what it is for ?
there are already fobj and function for this also operator %
and all other just directly use it liek div above ..

struct div_test
  {
  template<typename value_type>
  auto operator()(value_type const & tp)
    {
    return tp / tp;
    }
  };

Copy link
Owner

@arturbac arturbac May 9, 2025

Choose a reason for hiding this comment

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

@UF4007 FYI:

"OK, I rebased the fmod branch to the master branch and reverted some changes in the fmod"
Procedure for PR usually should look like:

  • fork
  • create in Your fork a branch for development
  • commit changes to this branch and submit PR from this branch
  • DO NOT submit changes from master branch, it complicates things for me to amend it and for You too in the future sync with master
    After it get squashed forget about that branch. I do not and many other devs do any merges, usually we squash PR into single commit as it gives better git history about changes in code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thus, before every time I decided to PR something, I must leave the old fork and reconstruct a new fork?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what it is for ?
there are already fobj and function for this also operator %
and all other just directly use it liek div above ..

struct div_test
  {
  template<typename value_type>
  auto operator()(value_type const & tp)
    {
    return tp / tp;
    }
  };

It is for comparison with the calculate speed of floating numbers. We cannot use operator% on floating numbers like 3.14%2.72. I don't want to make things complicated so I make the temporary "fixed_t" fmod function for benchmark use.

@UF4007
Copy link
Contributor Author

UF4007 commented May 10, 2025

Updated.
Remove the static function.

@UF4007 UF4007 requested a review from arturbac May 10, 2025 10:13
@arturbac arturbac merged commit fd640c0 into master May 10, 2025
4 checks passed
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.

3 participants