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

NAG fixes for fypp issue. #193

Closed
wants to merge 3 commits into from
Closed

Conversation

DJDavies2
Copy link
Contributor

Resovles #192.

The problem is that when the pre-processor expands the macros the result contains lines of longer than 132 characters due to the length of the file paths. This just adds some NAG specific code to circumvent the NAG compile issues described in #192. It isn't a very good solution but I can't obviously see anything better.

@FussyDuck
Copy link

FussyDuck commented May 8, 2024

CLA assistant check
All committers have signed the CLA.

@wdeconinck
Copy link
Member

I was just reading this here, which may solve this problem and be also a more desired output of handling _FILE_ anyway
https://fypp.readthedocs.io/en/stable/fypp.html#rendering-file-names-as-relative-paths
I can have a look on adding this flag --file-var-root to fypp generation stage.

@wdeconinck
Copy link
Member

wdeconinck commented Jun 6, 2024

It doesn't seem to be so straightforward to apply a general approach with '--file-var-root'.
However I verified the generated file <atlas-build-dir>/src/atlas_f/field/atlas_Field_module.F90
and I don't get the line-length going over 132, and fypp has split the line in 2, e.g.:

  call fckit_exception%abort( "Rank mismatch", "/Users/willem/work/atlas-bundle/source/atlas/src/atlas_f/field/atlas_Field_module.f&
# 172 "/Users/willem/work/atlas-bundle/source/atlas/src/atlas_f/field/atlas_Field_module.fypp"
      &ypp", 172 )

Could you show me from the generated file as well an example of offending line?

@DJDavies2
Copy link
Contributor Author

It doesn't seem to be so straightforward to apply a general approach with '--file-var-root'. However I verified the generated file <atlas-build-dir>/src/atlas_f/field/atlas_Field_module.F90 and I don't get the line-length going over 132, and fypp has split the line in 2, e.g.:

  call fckit_exception%abort( "Rank mismatch", "/Users/willem/work/atlas-bundle/source/atlas/src/atlas_f/field/atlas_Field_module.f&
# 172 "/Users/willem/work/atlas-bundle/source/atlas/src/atlas_f/field/atlas_Field_module.fypp"
      &ypp", 172 )

Could you show me from the generated file as well an example of offending line?

This is basically what I am seeing as well. I think my description of the issue in the first comment isn't quite correct, the line is being chopped at 132 characters but the problem is the insertion of the # 172 ... line in the middle of the string line break. It looks like NAG doesn't handle this syntax (it isn't standard).

@wdeconinck
Copy link
Member

Thanks for that clarification @DJDavies2 .
Perhaps you can try to add FYPP_ARGS --line-numbering-mode=nocontlines within this statement ?
-> https://github.com/ecmwf/atlas/blob/develop/src/atlas_f/CMakeLists.txt#L279
And verify the continuation line "line markers" are gone?
If that is the case I can add this argument within the fypp macro within fckit for NAG compiler so that it would just work in any fypp file, not just Atlas.

@DJDavies2
Copy link
Contributor Author

FYPP_ARGS --line-numbering-mode=nocontlines does seem to work for the atlas_f subdirectory. However there is also the file src/tests/runtime/fctest_trace.fypp and it isn't clear where I should make the change, can you point me to what I need to do?

@wdeconinck
Copy link
Member

Thanks for confirming it works. No need to change anything anymore.
I will make the change in the fypp cmake macro exported by fckit. That should then automatically fix atlas and other packages using fypp with NAG compiler.

@wdeconinck
Copy link
Member

That commit message in ecmwf/fckit@613d53e auto-closed this PR. Interesting.... Please verify this works for you :)

@DJDavies2
Copy link
Contributor Author

Yes, that changed worked for me, thanks.

@DJDavies2 DJDavies2 deleted the bugfix/192 branch June 10, 2024 15:35
wdeconinck added a commit to ecmwf/fckit that referenced this pull request Jun 10, 2024
* release/0.13.0:
  Version 0.13.0
  Remove line markers within line continuations added by fypp for NAG compiler (fixes ecmwf/atlas#193)
  GitHub ci notifications (#42)
  Upgrade fypp installation to pep517
  Fix copyright header dates and replace message with ecbuild_info
  Make python virtual environment installation optional
  Downgrade minimum cmake to 3.17
  Disable pip upgrade warnings
  Restore fckit-eval script processing of fypp
  Add tests for fckit_yaml_reader
  VENV: install python virtualenv with ruamel and fypp
  Increase CMake version to 3.20 to use cmake_path utils
  YAML reader: create minimal yaml reader class that exports the same API as pyyaml
  RUAMEL: add contributed ruamel.yaml repo
  Fix memory leak in fckit_configuration%get_config_list (Thanks Mirco)
@wdeconinck
Copy link
Member

This is now released in fckit 0.13.0

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

Successfully merging this pull request may close these issues.

3 participants