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

Fixed to reflect FLUKA-CERN #677

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

Conversation

makeclean
Copy link
Contributor

Fixes to FluDAG to allow building against the new FLUKA-CERN release. However... there has been a change to how FLUKA is run, originally (effectively hidden within the rfluka script) what happens is
flukahp input
where as what used to happen was
flukahp < input
No big issue, but before there were never any arguments to the fluka executable, and now there are, again no big issue, however this appears to have broken how FluDAG (and FluGG) would be run with a partial FLUKA input file (+ geometry supplied in memory). The subtle change, means there must be another function called which reads the input deck, before the flukam subroutine is called, when I get source code it'll be easy to figure out, currently working on getting that, in the mean time im scrabbling around trying to find it.

@gonuke
Copy link
Member

gonuke commented Jun 12, 2020

Does this mean that this PR is not ready until you get a chance to test that???

@makeclean
Copy link
Contributor Author

Ok, this is fixed now, and working, a few changes to documentation and this should be good to go.

@ljacobson64
Copy link
Member

Does rfluka.patch need to be changed in this PR as well?

@ljacobson64
Copy link
Member

In previous version of FLUKA, we apply a patch to rfluka.patch in order to have FLUKA recognize the -d option as specifying the DAGMC geometry file. However in FLUKA 4.1.0, the -d option is already reserved for a different purpose.

@makeclean
Copy link
Contributor Author

makeclean commented Jan 13, 2021 via email

@makeclean
Copy link
Contributor Author

makeclean commented Jan 13, 2021 via email

@makeclean
Copy link
Contributor Author

Alright, I think this is finally ready for merging. The documentation has been updated, the build is better. A new argument has been added and the patch file is no longer needed.

@makeclean
Copy link
Contributor Author

Looks like a style guide failure, the website lists the old astyle options, what are the current set of style guide checks that should be done? We should update the docs there too.

@gonuke
Copy link
Member

gonuke commented Jul 8, 2021

We moved to clang-format - we'll work on the docs

@gonuke
Copy link
Member

gonuke commented Jul 8, 2021

@gonuke
Copy link
Member

gonuke commented Jul 23, 2021

@makeclean - we're getting ready for another release. Might be good to include this if we can get the style check working. I'm going to close/reopen to kickstart the CI since we've transitioned to Github actions.

@gonuke gonuke closed this Jul 23, 2021
@gonuke gonuke reopened this Jul 23, 2021
@gonuke
Copy link
Member

gonuke commented Jul 23, 2021

Sorry, but we've also changed the News/Changelog requirement. I'll take care of that for you with a PR to this branch once you've rebased. 😊

@gonuke
Copy link
Member

gonuke commented Aug 13, 2021

Ping @makeclean for a quick rebase and switch from News => Changelog

@makeclean
Copy link
Contributor Author

Time has moved on, and so did - what did need to do to get this into main?

@gonuke
Copy link
Member

gonuke commented Feb 11, 2023

I think the first step is to get CI to run again.... I am not sure why I can't just ask it to rerun the tests?

@gonuke
Copy link
Member

gonuke commented Jul 20, 2023

Revisiting this @makeclean - we have a potential user of this who is interested to see it working. Probably needs a rebase to relaunch testing.

@ahnaf-tahmid-chowdhury
Copy link
Member

It seems that the next step is to create some test files for CI and see how they work, right?

@ahnaf-tahmid-chowdhury
Copy link
Member

@makeclean, can you please consider giving this a rebase?

@makeclean
Copy link
Contributor Author

Finally got around to refreshing this :)

@ahnaf-tahmid-chowdhury
Copy link
Member

Please update the CHANGELOG.rst and run clang-format in the root dir. It seems, windows is not downloading the PyNE sources.

@makeclean
Copy link
Contributor Author

Update the changlog to what?

@ahnaf-tahmid-chowdhury
Copy link
Member

We generally track our PRs through the changelog file located at doc/CHANGELOG.rst. Additionally, we use Google Clang Style for formatting our source code.

I have recently created a PR with these changes on your branch. Please take a look.

@makeclean
Copy link
Contributor Author

Ok, I think ready for review - I guess the windows build is a common failure?

@gonuke
Copy link
Member

gonuke commented Jul 18, 2024

Sadly, this is the first clear sign of the Windows failure, but it's not your fault... it seems to be related to some change in the underlying Windows runner. Hopefully, we can find a fix quickly....

@ahnaf-tahmid-chowdhury
Copy link
Member

I think the windows build issue is now solved by #958.

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.

4 participants