[TGraphErrors] add AddPointError method

[arabusov]

This Pull request:

Extends the functionality of TGraphErrors class on the manner of the TGraph class such that one does not need to prepare arrays with data before creating the TGraphErrors class and simply add points one-by-one.

Changes or fixes:

The pull request adds a new method to the class TGraphErrors called AddPointError

Checklist:

• [x] tested changes locally
• [x] updated the docs (added a doxygen line)

Code to test functionality:

(run as a root script)

void apge() {
    TCanvas *c1 = new TCanvas("c1","A Simple Graph Example",200,10,700,500);

    c1->SetGrid();

    const Int_t n = 20;
    TGraphErrors *gr = new TGraphErrors;
    for (Int_t i=0;i<n;i++) {
        Double_t x = i*0.1;
        gr->AddPointError(i*0.1, 10*sin(x+0.2), 0.3/(x*x+3.), x*x/(x*x + 8.));
    }
    gr->SetTitle("a simple graph");
    gr->GetXaxis()->SetTitle("X title");
    gr->GetYaxis()->SetTitle("Y title");
    gr->Draw("AP");

    // TCanvas::Update() draws the frame, after which one can change it
    c1->Update();
    c1->GetFrame()->SetBorderSize(12);
    c1->Modified();
}

[github-actions[bot]]

Test Results

    12 files      12 suites   2d 20h 15m 50s :stopwatch:  2 637 tests  2 637 :white_check_mark: 0 :zzz: 0 :x: 29 952 runs  29 952 :white_check_mark: 0 :zzz: 0 :x:

Results for commit e9a61ac0.

:recycle: This comment has been updated with latest results.

[arabusov]

@ferdymercury it seems I didn't follow the coding convention of root, what would be the best way to fix it?

[ferdymercury]

@ferdymercury it seems I didn't follow the coding convention of root, what would be the best way to fix it?

The instructions are here at the bottom: https://github.com/root-project/root/actions/runs/8688050309/job/23860940934?pr=15232

You might need to remove the -7 from the suggested command

[couet]

Similarly, we have also AddPoint for TGraph2D https://root.cern/doc/master/classTGraph2D.html#ae0c441531e5965c321335bb60df8101a, so why not also add AddPointErrrors to TGraph2DErrors https://root.cern/doc/master/classTGraph2DErrors.html ?

[couet]

And also, if you look at the inheritance diagram of TGraph:

you will notice that TGraphAsymmErrors TGraphBentErrors and TGraphMultiErrors will miss this new method. To be complete it should be added there too. Similarly, it should be also added in TGrpha2DAssymErrors

[arabusov]

@couet I found it's not quite obvious how to modify the TGraphMultiErrors class without modifying the SetPointError family methods. For TGraphAsymmErrors, TGraphBentErrors, and TGraph2DErrors's I didn't experience any problems to update them.

[ferdymercury]

@couet I found it's not quite obvious how to modify the TGraphMultiErrors class without modifying the SetPointError family methods. For TGraphAsymmErrors, TGraphBentErrors, and TGraph2DErrors's I didn't experience any problems to update them.

What error do you encounter if you try with:

void TGraphMultiErrors::AddPointError(Double_t x, Double_t y, Int_t ne, Double_t exL, Double_t exH, const Double_t *eyL, const Double_t *eyH) {
   AddPoint(x,y);
   SetPointError(fNpoints - 1, ne, exL, exH, eyL, eyH);
}

[arabusov]

@couet I found it's not quite obvious how to modify the TGraphMultiErrors class without modifying the SetPointError family methods. For TGraphAsymmErrors, TGraphBentErrors, and TGraph2DErrors's I didn't experience any problems to update them.

What error do you encounter if you try with:

void TGraphMultiErrors::AddPointError(Double_t x, Double_t y, Int_t ne, Double_t exL, Double_t exH, const Double_t *eyL, const Double_t *eyH) {
   AddPoint(x,y);
   SetPointError(fNpoints - 1, ne, exL, exH, eyL, eyH);
}

SetPointError calls SetPointEY which checks against fNYerrors and sets effectively all errors to zero (depends which constructor you call). One needs to call the AddYError first if the fNYerrors is below the number of requested errors.

[couet]

do you think it is ready to be merged ?

[arabusov]

do you think it is ready to be merged ?

Multierror is a difficult beast to be tackled, it requires a lot of changes. I'm not sure whether it should be a part of this pr or it needs a separate one.

[couet]

Yes, leave TGraphMultiErrors on side for now

[arabusov]

Yes, leave TGraphMultiErrors on side for now

Then, it is more or less ready.

[couet]

There is still failures.

[ferdymercury]

There is still failures.

Maybe rebasing to current master helps with those.

[arabusov]

There is still failures.

Maybe rebasing to current master helps with those.

I checked alma9 build, it failed somewhere else if I understand the log correctly:

  [ 98%] Linking CXX executable emitGraphIndependent
  /usr/bin/ld: cannot find -lblas
  Scanning dependencies of target G__TMVAUtils
  Consolidate compiler generated dependencies of target G__TMVAUtils
  [ 98%] Building CXX object tmva/tmva/CMakeFiles/G__TMVAUtils.dir/G__TMVAUtils.cxx.o
  collect2: error: ld returned 1 exit status
  gmake[2]: *** [tmva/pymva/test/CMakeFiles/TestRModelParserPyTorch.dir/build.make:124: tmva/pymva/test/TestRModelParserPyTorch] Error 1
  gmake[1]: *** [CMakeFiles/Makefile2:70193: tmva/pymva/test/CMakeFiles/TestRModelParserPyTorch.dir/all] Error 2

Suspicious, that all but linux jobs failed. I have tested locally on my linux machine a month ago, and it worked.

[ferdymercury]

Suspicious, that all but linux jobs failed. I have tested locally on my linux machine a month ago, and it worked.

To solve it, go to your fork on GitHub, master branch, click on Sync Fork.

Then go to your terminal, git pull your master branch. Then checkout your branch add_point. Call git rebase --interactive master

Finally, call git push -f add_point to end the rebase.

[arabusov]

Suspicious, that all but linux jobs failed. I have tested locally on my linux machine a month ago, and it worked.

To solve it, go to your fork on GitHub, master branch, click on Sync Fork.

Then go to your terminal, git pull your master branch. Then checkout your branch add_point. Call git rebase --interactive master

Finally, call git push -f add_point to end the rebase.

Ok, now my master branch is in sync with the mainstream.

[ferdymercury]

Thanks!

Now it's just missing the part of moving the doxygen comment to the cxx

[arabusov]

Thanks!

Now it's just missing the part of moving the doxygen comment to the cxx

Shall I also change the TGraph.h? In this PR, I didn't touch it so far.

[ferdymercury]

Shall I also change the TGraph.h? In this PR, I didn't touch it so far.

Which line do you mean? I don't think you can move there anything, as it's an inline declaration.

[arabusov]

Shall I also change the TGraph.h? In this PR, I didn't touch it so far.

Which line do you mean? I don't think you can move there anything, as it's an inline declaration.

I see. I would move the implementation to the cxx file (for both TGraph and TGraph2D).

[ferdymercury]

I see. I would move the implementation to the cxx file (for both TGraph and TGraph2D).

No need. When the implementation is inline, Doxygen comment goes in the header, that's fine, otherwise the comment goes in the .cxx

[arabusov]

I fixed some formatting to make clang happy.

[ferdymercury]

Thanks a lot !

@couet all looks green now

[arabusov]

Thanks a lot !

@couet all looks green now

Thanks for your help!