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

Remove meshmodifiers and reconfigure problems/examples directory #68

Merged
merged 15 commits into from
Oct 23, 2020

Conversation

cticenhour
Copy link
Member

See idaholab/moose#15980

Tagging @csdechant for review

csdechant
csdechant previously approved these changes Oct 20, 2020
Copy link
Collaborator

@csdechant csdechant left a comment

Choose a reason for hiding this comment

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

Just a note, some of the input files in the Problem directory still uses MeshModifiers. If there is no rush for this PR, I can edit those files later tonight, or we can change those input files next PR.

@cticenhour
Copy link
Member Author

I can take a look at those. There isn't a crazy rush here, as there are many other projects that need to be fixed up before the MOOSE PR can move forward. Also looks as though there is a Mac issue I need to look at as well...I am surprised because the object I removed wasn't used by any of the tests any longer.

I've been a proponent of removing the problems directory altogether, but I can see its value as an easy starting point if we are smart about it. I propose making symlinks in that directory (and perhaps renaming it to examples) to their corresponding tests. Then we can avoid having to worry about changes as that occurs naturally via test modifications. I've done this in my SPS work, and it works well.

Tagging @lindsayad and @keniley1 for their thoughts. I don't mind rolling any changes into this PR.

@moosebuild
Copy link
Collaborator

Job Mac Test on 92c184c : invalidated by @cticenhour

Compiler doesn't support openmp?

@lindsayad
Copy link
Member

lindsayad commented Oct 20, 2020 via email

@cticenhour
Copy link
Member Author

Many, if not all, of them already have test specs AFAIK - we just have to remember to hop up and make the same change we make to their tests. That's why I suggested the symlink. Any particular reason why you don't want that? It really helped us organize in Freya.

@lindsayad
Copy link
Member

lindsayad commented Oct 20, 2020 via email

@cticenhour
Copy link
Member Author

cticenhour commented Oct 20, 2020

Thanks for your thoughts. I'm very happy to remove it altogether as I've mentioned in the past - just wanted to throw an alternative out there to get some feedback. We wanted to make it obvious in Freya what were the best examples for functionality, since the tests we have can get quite granular. Zapdos is a bit different, since many of our tests are fully-formed problems. I'm going to go through the current problems directory and see what might need to be moved over to tests and what is duplicated.

@lindsayad
Copy link
Member

lindsayad commented Oct 20, 2020 via email

@cticenhour cticenhour marked this pull request as draft October 20, 2020 18:44
@cticenhour cticenhour changed the title Remove meshmodifiers Remove meshmodifiers and reconfigure problems/examples directory Oct 20, 2020
@cticenhour
Copy link
Member Author

cticenhour commented Oct 22, 2020

OK.... so there's a ton here. problems still exists, but remaining within is only @csdechant's acceleration work examples, non-accelerated examples, and a @keniley1's CRANE example for the AD action. Tests should be made for these, perhaps, for inclusion in an eventual examples directory, but I am pushing that to the next PR, since the MeshModifiers stuff needs to be removed sooner rather than later.

A summary of the rest of the changes:

  • All Schottky and reflections input files have been given Syntax Checks, as exodiff tests were never made for them.
  • All troubleshooting and test Python scripts in problems have been removed. ToCSV.py has been preserved and moved to scripts.
  • MeshModifiers documentation pages within Zapdos have been removed.
  • New tests have been created:
    • thesis_example -- a "coffee creamer" MOOSE example from @lindsayad's thesis. Let me know if you want me to get rid of it, but since it was mentioned, I figured it should remain somewhere user-accessible.
    • water_only [syntax check] -- this was a simple input file that showed usage of the Water Material. As this object is tested elsewhere, I didn't think another exodiff was necessary, but a simple usage example should be preserved.
  • Out-of-date duplicates of mean_en, TM, and current_carrying_wire were removed.
  • Added missing geo files to their relevant tests for reference.
  • Removed orphaned mesh files in problems.
  • Removed old troubleshooting inputs.
  • Removed SigmaMat and untested input file in favor of ADSurfaceCharge.

Whew....I think that's it. Please let me know if you all would like me to squash this down a bit.

Copy link
Collaborator

@csdechant csdechant left a comment

Choose a reason for hiding this comment

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

Everything looks good to me. Unless anyone has any comments, I think this PR is ready to go.

@cticenhour
Copy link
Member Author

Sounds good - I'll give folks a chance to comment and merge late tonight or in the morning.

@cticenhour cticenhour merged commit ffda2e5 into shannon-lab:devel Oct 23, 2020
@cticenhour cticenhour deleted the remove-meshmodifiers branch October 23, 2020 02:34
@cticenhour cticenhour mentioned this pull request Oct 26, 2020
3 tasks
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.

None yet

4 participants