Skip to content

Set precision for double in FileStorage#25087

Closed
Kumataro wants to merge 3 commits intoopencv:4.xfrom
Kumataro:fix25073-2
Closed

Set precision for double in FileStorage#25087
Kumataro wants to merge 3 commits intoopencv:4.xfrom
Kumataro:fix25073-2

Conversation

@Kumataro
Copy link
Copy Markdown
Contributor

@Kumataro Kumataro commented Feb 24, 2024

Close #25073

I feel the function/enum names may have to be improved.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@asmorkalov asmorkalov requested a review from vpisarev February 25, 2024 07:41
@Kumataro
Copy link
Copy Markdown
Contributor Author

Kumataro commented Feb 25, 2024

Some tests had been failed, however I think they are false-positive.

Window10-x86 BuidAndTest

https://github.com/opencv/opencv/actions/runs/8034792429/job/21950114758#step:22:184

[----------] 21 tests from Imgcodecs_Tiff
[ RUN ] Imgcodecs_Tiff.decode_tile16384x16384
C:\GHA-OCV-4_work\opencv\opencv\opencv\modules\imgcodecs\test\test_tiff.cpp(48): error: Expected equality of these values:
0
remove(file4.c_str())
Which is: -1

It means failed to remove temporary file.
So I think this reason may be test-environment.

macOS-x64/BuildAndTest

https://github.com/opencv/opencv/actions/runs/8034792429/job/21950115658#step:13:1699

Error: The action has timed out.

This is timeout error.
So I think this reason may be test-environment.

macOS-x64/BuildContrib

https://github.com/opencv/opencv/actions/runs/8034792429/job/21950115851#step:11:2415

Error: The action has timed out.

This is timeout error.
So I think this reason may be test-environment.

Android-Test/BuildAndTest

https://github.com/opencv/opencv/actions/runs/8034792429/job/21950114939#step:15:4916

SEVERE: Execute org.opencv.test.core.MatTest.testElemSize: FAILED

opencv/modules/core/misc/java/test/MatTest.java

Lines 255 to 259 in ae347ab

public void testElemSize() {
assertEquals(Byte.SIZE / 8 * gray0.channels(), gray0.elemSize());
assertEquals(Float.SIZE / 8 * gray0_32f.channels(), gray0_32f.elemSize());
assertEquals(Byte.SIZE / 8 * rgbLena.channels(), rgbLena.elemSize());
}

This test is to check element length, but It is not related with this patch.
So I think this reason may be test-environment.

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Apr 3, 2024 •
edited
Loading

@Kumataro Thanks a lot for research and contribution! The PR was discussed on core team meeting and we propose to change approach. C++ standard proposes %g option to shortest representation of the value in printf-like functions. It'll be %e or %f depending on value. It covers the original issue without API changes and always produces efficient result. See https://cplusplus.com/reference/cstdio/printf/

@Kumataro
Copy link
Copy Markdown
Contributor Author

Kumataro commented Apr 3, 2024

I agree with the results of the above review.

Currently, there is a conflict with this commit, and a new and different approach is required, so I would like to withdraw this pull request.

Thank you very much for your consideration!

@Kumataro Kumataro closed this Apr 3, 2024
@Kumataro Kumataro mentioned this pull request Apr 6, 2024
Merged
6 tasks
@Kumataro
Copy link
Copy Markdown
Contributor Author

Kumataro commented Apr 6, 2024

I created new pull request using format %g at #25351

asmorkalov pushed a commit that referenced this pull request Apr 10, 2024
@Kumataro
Merge pull request #25351 from Kumataro:fix25073_format_g
b14ea19 
core: persistence: output reals as human-friendly expression. #25351

Close #25073
Related #25087

This patch is need to merge same time with opencv/opencv_contrib#3714

### Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [x] There is a reference to the original bug report and related work
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
susumu-iino pushed a commit to susumu-iino/opencv that referenced this pull request Apr 11, 2024
@Kumataro @susumu-iino
Merge pull request opencv#25351 from Kumataro:fix25073_format_g
a90db30 
core: persistence: output reals as human-friendly expression. opencv#25351

Close opencv#25073
Related opencv#25087

This patch is need to merge same time with opencv/opencv_contrib#3714

### Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [x] There is a reference to the original bug report and related work
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
klatism pushed a commit to klatism/opencv that referenced this pull request May 17, 2024
@Kumataro @klatism
Merge pull request opencv#25351 from Kumataro:fix25073_format_g
4fd3898 
core: persistence: output reals as human-friendly expression. opencv#25351

Close opencv#25073
Related opencv#25087

This patch is need to merge same time with opencv/opencv_contrib#3714

### Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [x] There is a reference to the original bug report and related work
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vpisarev vpisarev Awaiting requested review from vpisarev

Assignees

No one assigned

Labels

category: core feature pr: Discussion Required

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Set precision for double in FileStorage

2 participants

@Kumataro @asmorkalov