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.

Sign up for GitHub

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

Jump to bottom

Make Revisions obsolete, and sync hpcgap/lib/init.g #1734

Merged
merged 4 commits into from
Sep 21, 2017
Merged

Make Revisions obsolete, and sync hpcgap/lib/init.g #1734

merged 4 commits into from
Sep 21, 2017

Conversation

fingolfin
Copy link
Member

This might impact PR #1714 (not in a major way, but enough to create a conflict), and also PR #1710 resp #1729

@fingolfin fingolfin added topic: HPC-GAP Issues and PRs related to HPC-GAP release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Sep 20, 2017
@fingolfin fingolfin added topic: HPC-GAP unification and removed topic: HPC-GAP Issues and PRs related to HPC-GAP labels Sep 20, 2017
@codecov
Copy link

codecov bot commented Sep 20, 2017 •
edited
Loading

Codecov Report

Merging #1734 into master will increase coverage by 0.01%.
The diff coverage is 53.65%.

@@            Coverage Diff             @@
##           master    #1734      +/-   ##
==========================================
+ Coverage    64.5%   64.52%   +0.01%     
==========================================
  Files        1001     1000       -1     
  Lines      326384   326028     -356     
  Branches    13239    13216      -23     
==========================================
- Hits       210538   210356     -182     
+ Misses     112989   112812     -177     
- Partials     2857     2860       +3
Impacted Files Coverage Δ
lib/obsolete.gd 28.35% <ø> (ø) ⬆️
lib/hpc/thread1.g 54.34% <ø> (+4.34%) ⬆️
lib/session.g 90.32% <100%> (ø) ⬆️
lib/init.g 45.16% <44.11%> (-0.13%) ⬇️
lib/files.gi 46.72% <0%> (-7.38%) ⬇️
src/vars.h 93.93% <0%> (-3.04%) ⬇️
lib/files.gd 61.85% <0%> (-1.04%) ⬇️
src/system.c 52.94% <0%> (-0.34%) ⬇️
hpcgap/lib/hpc/stdtasks.g 38.61% <0%> (-0.26%) ⬇️
src/hpc/thread.c 46.44% <0%> (-0.2%) ⬇️
... and 5 more

@fingolfin
Copy link
Member Author

Note that this PR also removes some MPI leftovers from init.g, including the line ReadLib("distributed/distgap.g");

I just split the removal of hpcgap/lib/init.g into two commits -- the final commit only removes the now-identical file. But by looking at the middle commit, you can see what exactly was changed when merging the two copies of init.g.

@olexandr-konovalov
Copy link
Member

I see that KERNEL_VERSION and PACKAGE_VERSIONS are note used in any of the redistributed packages - is it time to get rid of them?

olexandr-konovalov
olexandr-konovalov approved these changes Sep 20, 2017
View reviewed changes
Copy link
Member

@olexandr-konovalov olexandr-konovalov left a comment

Choose a reason for hiding this comment

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

Looks good - left a couple of minor comments. Thanks.

hpcgap/lib/init.g Outdated
if libs <> [] then
print_info( " Libs used: ", libs, "\n" );
print_info( " Configuration: ", libs, "\n" );
Copy link
Member

@olexandr-konovalov olexandr-konovalov Sep 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Any text justifying this renaming?

Copy link
Member Author

Choose a reason for hiding this comment

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

Justification: Syncing changes between GAP and HPC-GAP is literally what this PR is about... :-)

I.e. this was changed already quite some time ago in GAP, because we now list more than list libs. The variable libs should be renamed to something better eventually, but I deliberately did not, as this was about syncing the changes, not about cleaning init.g up.

hpcgap/lib/init.g Outdated
if IsBoundGlobal("MPI_Initialized") then
ReadLib("distributed/distgap.g");
if IsBound(HPCGAP) then
HELP_ADD_BOOK("HPC-GAP", "HPC-GAP Shared Memory Extensions and MPI", "doc/hpc");
Copy link
Member

Choose a reason for hiding this comment

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

Please fix the 2nd argument - there is no MPI there any more.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, fixed

@fingolfin
Copy link
Member Author

Yes, I think it would be good to get rid of KERNEL_VERSION and PACKAGE_VERSIONS -- but that's for another PR. Perhaps you can make one for it once this PR is merged?

@fingolfin
Copy link
Member Author

If you want to remove unused obsoletes, there are a bunch others marked as "not used". And several which aren't used anymore:I last went through that file in January 2016, some things changed since then, e.g. in xgap, so FactorCosetOperation or TeXObj (but LaTeXObj is used in lpres... just like a bunch of other obsolete things :-(

@olexandr-konovalov olexandr-konovalov merged commit 92e5583 into gap-system:master Sep 21, 2017
@fingolfin fingolfin deleted the mh/hpcgap-init branch September 26, 2017 00:45
@olexandr-konovalov olexandr-konovalov mentioned this pull request Oct 6, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olexandr-konovalov olexandr-konovalov olexandr-konovalov approved these changes

@ChrisJefferson ChrisJefferson Awaiting requested review from ChrisJefferson

@markuspf markuspf Awaiting requested review from markuspf

Assignees
No one assigned
Labels
release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: HPC-GAP unification
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fingolfin @olexandr-konovalov