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.

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

Hopefully, fix the long-standing problem with h2root #15915

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

couet
Copy link
Member

@couet couet commented Jun 24, 2024

Using the C option (meaning C I.O) in hropen seems to fix the issue on Mac.

@couet couet requested a review from dpiparo June 24, 2024 13:06
@couet couet self-assigned this Jun 24, 2024
@couet couet requested a review from pcanal as a code owner June 24, 2024 13:06
Copy link

github-actions bot commented Jun 24, 2024

Test Results

    18 files      18 suites   4d 11h 9m 38s ⏱️
 2 682 tests  2 680 ✅ 0 💤 2 ❌
46 556 runs  46 553 ✅ 0 💤 3 ❌

For more details on these failures, see this check.

Results for commit e052758.

♻️ This comment has been updated with latest results.

@hahnjo
Copy link
Member

hahnjo commented Jun 25, 2024

As far as I can tell, this shouldn't make a difference because HROPEN defaults to to C unless F is specified:

IF (INDEX(CHOPT,'F').EQ.0) THEN
IC = MIN(LENOCC(CHOPT)+1,8)
CHOPT(IC:IC) = 'C'
ENDIF

@couet
Copy link
Member Author

couet commented Jun 27, 2024

@hahnjo it made on on MacOS. (long debug)

@dpiparo dpiparo closed this Sep 9, 2024
@dpiparo dpiparo reopened this Sep 9, 2024
@hahnjo
Copy link
Member

hahnjo commented Sep 9, 2024

my comment from #15915 (comment) still stands: based on the code, there should be no change in behavior because C is already the default.

@couet
Copy link
Member Author

couet commented Dec 19, 2024

I know "c" is supposed to be the default, but this change made a difference on Mac when I debugged it a while ago. I guess it is worth merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants