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

Aspect 1.5 #2190

Closed
wants to merge 3 commits into from
Closed

Aspect 1.5 #2190

wants to merge 3 commits into from

Conversation

unfelipe
Copy link

Hi all,

     After inspecting Aspect manual 1.5.0; and confirming with Wolfgang, there seems to be a small error on the manual description of the material model 'simple', particularly in the temperature-dependent viscosity. Being emphatic: the code is right, it's is only the handbook writing that needs a correction.

 The issue is on the explanation of the exponential viscosity function via H(x), where the written thresholds are 0.01 and 100, contradicting the arbitrary user-defined thermal prefactors tau_min, tau_max, which would potentially lead to discontinuities or empty intervals if the code worked that way. 

 I think the manual should write:

 H(x) = {
                    tau_min    if  x < tau_min

                     x   if   tau_min <= x <= tau_max                 (*This would be the corrected text line)            

                    tau_max   if  x > tau_max  
            
              }

warm regards,
Felipe

@naliboff
Copy link
Contributor

Hi Felipe - Thanks for finding and pointing out the incorrect documentation! However, I don't see the commit with the changes you describe?

Also, would you mind adding the changes from a branch off of the current developer version? This would significantly reduce the number of merge conflicts. If you would like, I can provide some step-by-step instructions on how to do this. If it's easier on your side, also happy to submit a pull request with the fix you outline above.

Thanks again!

@gassmoeller
Copy link
Member

Hi @unfelipe, I do not think this pull request is what you intended. You opened a pull request from the main release branch aspect-1.5 to the current development branch master. However, you likely wanted to open a pull request from a branch in your repository (with some changes) to the development branch. Have a look here for an explanation of the process. Let us know as a comment or via mail if you get stuck. I will close this PR for now, since it is a bit confusing (and I do not want to risk that somebody merges this PR).

@naliboff
Copy link
Contributor

@gassmoeller - Thanks for closing this, was going to do so today. @unfelipe and I have been discussing offline about how to do a pull request.

@gassmoeller
Copy link
Member

Wow, this PR demonstrates an absolutely dangerous feature of github. To all developers with write access to the repository who see this: Do not press the 'Delete branch' button when somebody opens a pull request from the main repo to the main repo. You might delete a branch we want to keep (like the 1.5 release branch in this case). While it is fixable (the release tag will stay around), it will cause some trouble. I did not know this would be offered by Github.

@naliboff
Copy link
Contributor

Yikes. Also surprised that option is offered as a feature in pull requests, especially from users without write access.

@bangerth
Copy link
Contributor

bangerth commented May 1, 2018

How interesting -- that's about as insidious as phishing attacks with forged web sites!

@gassmoeller
Copy link
Member

@tjesser-ucdavis-edu had the proposal to just make every release branch a protected branch. Maybe that would remove the button, something to try.

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

5 participants