new test_trired test; most KeywordSet() with numeric fixed; -DEIGEN3=off now OK; limited cleaning in eigenvalues_solvers.*pp

alaingdl commented 7 months ago

just adding #include in typedef.cpp is enough to be able to compile "everywhere" when Eigen is turn OFF
copyright headers of the last files written by Gilles ... (eigenvalues_solvers.*pp)
very limited cleaning in those files (eg having Ix for the keywords index for easier reading (convention))
better management of Eigen/GSL for these last codes
When GDL compiled without Eigen ON, nice message as some other codes (LA_LEAST_SQUARES ...)
```
GDL> la_elmhes()
% GDL compiled without Eigen3 : LA_ELMHES not available
   0
```
All remaining KeywordSet() with numerical values should have been cleaned, except in HDF and NetCDF files (may be I missed the case for
new test for TRIRED/TRIQL (test_trired.pro added in LIST)

What is not OK now from my point of view: LA_HQR missing, LA_TRIQL missing, still pending bugs in CHOLESKY codes

GillesDuvert commented 7 months ago

needed to remove test_trired.pro from LIST in master

alaingdl commented 7 months ago

needed to remove test_trired.pro from LIST in master

Sorry for that, this PR became more complex than expected, since the OSX is unexpectedly broken now :(( (and I cannot reproduce locally the problem as report on CI)

GillesDuvert commented 7 months ago

Actually I'm perplexed by the CI results on #1819 where:

TEST_OBJ_NEW calls for IDLSYSMONITORINFO__DEFINE and does not find it (OK on my machine)
TEST_FORMAT in error although the #1819 PR is made to solve the problem (and it does, on my machine)
On windows, TEST_TICK_GET fails because the driver is not found (meaning: plplot installation is bad again)
On OSX, everything fails due to OpenMPI (and I fail to see why we enforce OpenMPI on Windows and Mac since those machines/os are never in clusters)

Plus, the strange impression that concurrent versioning as we do todays does not report all conflicts and that we sometimes get regressions.

alaingdl commented 7 months ago

Actually I'm perplexed by the CI results on #1819 where:

1. TEST_OBJ_NEW calls for IDLSYSMONITORINFO__DEFINE and does not find it (OK on my machine)

2. TEST_FORMAT in error although the [closes #1706 closes #1811 #1819](https://github.com/gnudatalanguage/gdl/pull/1819) PR is made to solve the problem (and it does, on my machine)

3. On windows, TEST_TICK_GET fails because the driver is not found (meaning: plplot installation is bad again)

4. On OSX, everything fails due to OpenMPI (and I fail to see why we enforce OpenMPI on Windows and Mac since those machines/os are never in clusters)

Plus, the strange impression that concurrent versioning as we do todays does not report all conflicts and that we sometimes get regressions.

the same for me !

the messages for all test on OSX is related to strange mmap ? Do we change somethings, Do they change somethings

[Mac-1713890950329.local:21868] shmem: mmap: an error occurred while determining whether or not /var/folders/z4/sx941vhj3jd2jb9_gwftzctc0000gn/T//ompi.Mac-1713890950329.501/jf.0/1359872000/sm_segment.Mac-1713890950329.501.510e0000.0 could be created.

Unfortunately too busy today to spend time now on that, a pity my PR is not finished due to the OSX issue !

slayoo commented 7 months ago

On OSX, everything fails due to OpenMPI (and I fail to see why we enforce OpenMPI on Windows and Mac since those machines/os are never in clusters)

Moreover, we do not actually test anything under mpiexec - it's just compilation with MPI enabled, no runtime checks.

Plus, the strange impression that concurrent versioning as we do todays does not report all conflicts and that we sometimes get regressions.

@GillesDuvert, please elaborate/exemplify - will do my best to clarify.

GillesDuvert commented 7 months ago

@slayoo I closed #1800 without merging because it would not take correctly the changes I made in the .g files (and all the related, ANTLR-produced, FMT.pp files). It reported a conflict after @alaingdl merged my #1799, and this was useful, because I realized the above problem then. PR #1819, essentially the same, but rebased on the master version after #1799 has been merged, is OK (apart the other troubles arising in between due to #1812 to #1815) I'm not sure if this comes from 'squashing', and In fact I wonder ---never been interested in--- on what basis git can tell that one piece of code in a PR should replace another piece in another PR when those PRs are merged by several people at unrelated and non-monotonous times, which, in retrospect, would seem magical.

slayoo commented 7 months ago

@GillesDuvert

on what basis git can tell that one piece of code in a PR should replace another piece in another PR when those PRs are merged by several people at unrelated and non-monotonous times, which, in retrospect, would seem magical.

I'd say there is, and there cannot be, any magic here; merges and rebases report conflicts whenever the same line is modified, so "git can tell that one piece of code in a PR should replace another piece in another PR" works only for these pieces of code that were not modified.

@alaingdl, looking at the PRs, it seems you prefer to avoid branching, and instead work from one dev branch. This makes having multiple PRs from your fork impossible and also gives many people write-access to your master branch. But, it can still be made to work with the current squash strategy (avoiding seeing old commits in every PR) by rebasing your master against upstream/master before pushing, i.e.:

git remote add upstream git@github.com:gnudatalanguage/gdl.git
git fetch upstream
git rebase upstream/main  # (resolving any conflicts)
git push -f

HTH

alaingdl commented 7 months ago

@alaingdl, looking at the PRs, it seems you prefer to avoid branching, and instead work from one dev branch. This makes having multiple PRs from your fork impossible and also gives many people write-access to your master branch. But, it can still be made to work with the current squash strategy (avoiding seeing old commits in every PR) by rebasing your master against upstream/master before pushing, i.e.:
git remote add upstream git@github.com:gnudatalanguage/gdl.git
git fetch upstream
git rebase upstream/main  # (resolving any conflicts)
git push -f
HTH

as usual, thank you so much for clear explanations,, I will try to take it into account in my way to work. I was quite happy few months ago to have a new way to push codes into the master, but I miss the step for the new squash & merge !

A.

GillesDuvert commented 7 months ago

See comment for squash & merge in https://github.com/gnudatalanguage/gdl/commit/bdfbd875639828d5e1179ae63624965cec0f7f7a, reproduced here:

Don't see why the CI still crashes after the problems were "solved" . Code inspection do not show any problem, especially as the impacted files are not in conflict. I merge, and we'll eventually patch afterwards.

slayoo commented 7 months ago

To trigger a build of a PR against an updated master branch, we can either close-and-reopen a PR (likely simplest) or click on the "Update branch" button, or do a rebase from command line. More info here: https://github.blog/changelog/2022-02-03-more-ways-to-keep-your-pull-request-branch-up-to-date/ Here, it was apparently performing the checks against an outdated base branch.

GillesDuvert commented 7 months ago

@slayoo thanks obviously close+reopen was the thing to do.

GillesDuvert commented 7 months ago

... and indeed it got green light.

gnudatalanguage / gdl

new test_trired test; most KeywordSet() with numeric fixed; -DEIGEN3=off now OK; limited cleaning in eigenvalues_solvers.*pp #1816